From 2c342a42a1d6bafa4ac4c2dc76924e7ccf8a0955 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Thu, 9 Oct 2025 20:36:03 +0000 Subject: [PATCH 01/17] Add support for MIN time segment management in Growatt integration --- .../components/growatt_server/__init__.py | 195 ++++++++- .../components/growatt_server/const.py | 14 + .../components/growatt_server/coordinator.py | 155 +++++++ .../components/growatt_server/icons.json | 10 + .../components/growatt_server/services.yaml | 50 +++ .../components/growatt_server/strings.json | 56 ++- tests/components/growatt_server/conftest.py | 43 +- .../growatt_server/test_services.py | 395 ++++++++++++++++++ 8 files changed, 878 insertions(+), 40 deletions(-) create mode 100644 homeassistant/components/growatt_server/icons.json create mode 100644 homeassistant/components/growatt_server/services.yaml create mode 100644 tests/components/growatt_server/test_services.py diff --git a/homeassistant/components/growatt_server/__init__.py b/homeassistant/components/growatt_server/__init__.py index 6483e7a543c672..011b9c8ab47a1c 100644 --- a/homeassistant/components/growatt_server/__init__.py +++ b/homeassistant/components/growatt_server/__init__.py @@ -1,22 +1,31 @@ """The Growatt server PV inverter sensor integration.""" from collections.abc import Mapping +from datetime import datetime import logging import growattServer +import voluptuous as vol from homeassistant.const import CONF_PASSWORD, CONF_TOKEN, CONF_URL, CONF_USERNAME -from homeassistant.core import HomeAssistant -from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryError +from homeassistant.core import HomeAssistant, ServiceCall, SupportsResponse +from homeassistant.exceptions import ( + ConfigEntryAuthFailed, + ConfigEntryError, + HomeAssistantError, +) +from homeassistant.helpers import selector from .const import ( AUTH_API_TOKEN, AUTH_PASSWORD, + BATT_MODE_MAP, CONF_AUTH_TYPE, CONF_PLANT_ID, DEFAULT_PLANT_ID, DEFAULT_URL, DEPRECATED_URLS, + DOMAIN, LOGIN_INVALID_AUTH_CODE, PLATFORMS, ) @@ -185,9 +194,191 @@ async def async_setup_entry( # Set up all the entities await hass.config_entries.async_forward_entry_setups(config_entry, PLATFORMS) + # Register services for MIN/TLX devices + await _async_register_services(hass, config_entry, device_coordinators) + return True +async def _async_register_services( + hass: HomeAssistant, + config_entry: GrowattConfigEntry, + device_coordinators: dict, +) -> None: + """Register services for MIN/TLX devices.""" + # Get all MIN coordinators with V1 API - single source of truth + min_coordinators = { + coord.device_id: coord + for coord in device_coordinators.values() + if coord.device_type == "min" and coord.api_version == "v1" + } + + if not min_coordinators: + _LOGGER.debug( + "No MIN devices with V1 API found, skipping TOU service registration. " + "Services require MIN devices with token authentication" + ) + return + + _LOGGER.info( + "Found %d MIN device(s) with V1 API, registering TOU services", + len(min_coordinators), + ) + + def get_coordinator(device_id: str | None = None) -> GrowattCoordinator: + """Get coordinator by device_id with consistent behavior.""" + if device_id is None: + if len(min_coordinators) == 1: + # Only one device - return it + return next(iter(min_coordinators.values())) + # Multiple devices - require explicit selection + device_list = ", ".join(min_coordinators.keys()) + raise HomeAssistantError( + f"Multiple MIN devices available ({device_list}). " + "Please specify device_id parameter." + ) + + # Explicit device_id provided + if device_id not in min_coordinators: + raise HomeAssistantError(f"MIN device '{device_id}' not found") + + return min_coordinators[device_id] + + async def handle_update_min_time_segment(call: ServiceCall) -> None: + """Handle update_min_time_segment service call.""" + segment_id = call.data["segment_id"] + batt_mode_str = str(call.data["batt_mode"]) + start_time_str = call.data["start_time"] + end_time_str = call.data["end_time"] + enabled = call.data["enabled"] + device_id = call.data.get("device_id") + + _LOGGER.debug( + "handle_update_min_time_segment: segment_id=%d, batt_mode=%s, start=%s, end=%s, enabled=%s, device_id=%s", + segment_id, + batt_mode_str, + start_time_str, + end_time_str, + enabled, + device_id, + ) + + # Convert batt_mode string to integer + batt_mode = BATT_MODE_MAP.get(batt_mode_str) + if batt_mode is None: + _LOGGER.error("Invalid battery mode: %s", batt_mode_str) + raise HomeAssistantError(f"Invalid battery mode: {batt_mode_str}") + + # Convert time strings to datetime.time objects + try: + start_time = datetime.strptime(start_time_str, "%H:%M").time() + end_time = datetime.strptime(end_time_str, "%H:%M").time() + except ValueError as err: + _LOGGER.error("Start_time and end_time must be in HH:MM format") + raise HomeAssistantError( + "start_time and end_time must be in HH:MM format" + ) from err + + # Get the appropriate MIN coordinator + coordinator = get_coordinator(device_id) + + try: + await coordinator.update_min_time_segment( + segment_id, + batt_mode, + start_time, + end_time, + enabled, + ) + except Exception as err: + _LOGGER.error( + "Error updating MIN time segment %d: %s", + segment_id, + err, + ) + raise HomeAssistantError( + f"Error updating MIN time segment {segment_id}: {err}" + ) from err + + async def handle_read_min_time_segments(call: ServiceCall) -> dict: + """Handle read_min_time_segments service call.""" + # Get the appropriate MIN coordinator + coordinator = get_coordinator(call.data.get("device_id")) + + try: + time_segments = await coordinator.read_min_time_segments() + except Exception as err: + _LOGGER.error("Error reading MIN time segments: %s", err) + raise HomeAssistantError(f"Error reading MIN time segments: {err}") from err + else: + return {"time_segments": time_segments} + + # Create device selector schema helper + device_selector_fields = {} + if len(min_coordinators) > 1: + device_options = [ + selector.SelectOptionDict(value=device_id, label=f"MIN Device {device_id}") + for device_id in min_coordinators + ] + device_selector_fields[vol.Required("device_id")] = selector.SelectSelector( + selector.SelectSelectorConfig(options=device_options) + ) + + # Define service schemas + update_schema_fields = { + vol.Required("segment_id"): selector.NumberSelector( + selector.NumberSelectorConfig( + min=1, max=9, mode=selector.NumberSelectorMode.BOX + ) + ), + vol.Required("batt_mode"): selector.SelectSelector( + selector.SelectSelectorConfig( + options=[ + selector.SelectOptionDict(value="load-first", label="Load First"), + selector.SelectOptionDict( + value="battery-first", label="Battery First" + ), + selector.SelectOptionDict(value="grid-first", label="Grid First"), + ] + ) + ), + vol.Required("start_time"): selector.TimeSelector(), + vol.Required("end_time"): selector.TimeSelector(), + vol.Required("enabled"): selector.BooleanSelector(), + **device_selector_fields, + } + + read_schema_fields = {**device_selector_fields} + + # Register services + services_to_register = [ + ( + "update_min_time_segment", + handle_update_min_time_segment, + update_schema_fields, + ), + ("read_min_time_segments", handle_read_min_time_segments, read_schema_fields), + ] + + for service_name, handler, schema_fields in services_to_register: + if not hass.services.has_service(DOMAIN, service_name): + schema = vol.Schema(schema_fields) if schema_fields else None + supports_response = ( + SupportsResponse.ONLY + if service_name == "read_min_time_segments" + else SupportsResponse.NONE + ) + + hass.services.async_register( + DOMAIN, + service_name, + handler, + schema=schema, + supports_response=supports_response, + ) + _LOGGER.info("Registered service: %s", service_name) + + async def async_unload_entry( hass: HomeAssistant, config_entry: GrowattConfigEntry ) -> bool: diff --git a/homeassistant/components/growatt_server/const.py b/homeassistant/components/growatt_server/const.py index e2f04ad05f3d3a..d5b3405d575221 100644 --- a/homeassistant/components/growatt_server/const.py +++ b/homeassistant/components/growatt_server/const.py @@ -46,3 +46,17 @@ # Config flow abort reasons ABORT_NO_PLANTS = "no_plants" + +# Battery modes for TOU (Time of Use) settings +BATT_MODE_LOAD_FIRST = 0 +BATT_MODE_BATTERY_FIRST = 1 +BATT_MODE_GRID_FIRST = 2 + +BATT_MODE_MAP = { + "load-first": BATT_MODE_LOAD_FIRST, + "0": BATT_MODE_LOAD_FIRST, + "battery-first": BATT_MODE_BATTERY_FIRST, + "1": BATT_MODE_BATTERY_FIRST, + "grid-first": BATT_MODE_GRID_FIRST, + "2": BATT_MODE_GRID_FIRST, +} diff --git a/homeassistant/components/growatt_server/coordinator.py b/homeassistant/components/growatt_server/coordinator.py index 2f00c542c13377..e749dd2f108ede 100644 --- a/homeassistant/components/growatt_server/coordinator.py +++ b/homeassistant/components/growatt_server/coordinator.py @@ -12,6 +12,7 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_PASSWORD, CONF_URL, CONF_USERNAME from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed from homeassistant.util import dt as dt_util @@ -251,3 +252,157 @@ def get_data( self.previous_values[variable] = return_value return return_value + + async def update_min_time_segment( + self, segment_id: int, batt_mode: int, start_time, end_time, enabled: bool + ) -> None: + """Update a MIN inverter time segment. + + Args: + segment_id: Time segment ID (1-9) + batt_mode: Battery mode (0=load first, 1=battery first, 2=grid first) + start_time: Start time (datetime.time object) + end_time: End time (datetime.time object) + enabled: Whether the segment is enabled + """ + _LOGGER.debug( + "Updating MIN time segment %s for device %s", + segment_id, + self.device_id, + ) + + if self.api_version != "v1": + _LOGGER.warning( + "Updating time segments is only supported with V1 API (token authentication)" + ) + raise HomeAssistantError( + "Updating time segments requires token authentication" + ) + + try: + # Use V1 API for token authentication + response = await self.hass.async_add_executor_job( + self.api.min_write_time_segment, + self.device_id, + segment_id, + batt_mode, + start_time, + end_time, + enabled, + ) + except growattServer.GrowattV1ApiError as err: + _LOGGER.error( + "API error updating MIN time segment %s for device %s: %s", + segment_id, + self.device_id, + err, + ) + raise HomeAssistantError(f"API error updating time segment: {err}") from err + + # Check response error code + if response.get("error_code", 1) != 0: + error_msg = response.get("error_msg", "Unknown error") + _LOGGER.error( + "Failed to update MIN time segment %s for device %s: %s", + segment_id, + self.device_id, + error_msg, + ) + raise HomeAssistantError(f"Failed to update time segment: {error_msg}") + + _LOGGER.info( + "Successfully updated MIN time segment %s for device %s", + segment_id, + self.device_id, + ) + # Trigger a refresh to update the data + await self.async_refresh() + + async def read_min_time_segments(self) -> list[dict]: + """Read time segments from a MIN inverter. + + Returns: + List of dictionaries containing segment information + """ + _LOGGER.debug( + "Reading MIN time segments for device %s", + self.device_id, + ) + + if self.api_version != "v1": + _LOGGER.warning( + "Reading time segments is only supported with V1 API (token authentication)" + ) + raise HomeAssistantError( + "Reading time segments requires token authentication" + ) + + # Ensure we have current data + if not self.data: + _LOGGER.debug("Triggering refresh to get time segments") + await self.async_refresh() + + time_segments = [] + mode_names = {0: "Load First", 1: "Battery First", 2: "Grid First"} + + # Extract time segments from coordinator data + for i in range(1, 10): # Segments 1-9 + segment = self._parse_time_segment(i, mode_names) + time_segments.append(segment) + + _LOGGER.debug( + "Read %d time segments for device %s", len(time_segments), self.device_id + ) + return time_segments + + def _parse_time_segment(self, segment_id: int, mode_names: dict[int, str]) -> dict: + """Parse a single time segment from coordinator data.""" + # Get raw time values + start_time_raw = self.data.get(f"forcedTimeStart{segment_id}", "0:0") + end_time_raw = self.data.get(f"forcedTimeStop{segment_id}", "0:0") + + # Handle 'null' or empty values + if start_time_raw in ("null", None, ""): + start_time_raw = "0:0" + if end_time_raw in ("null", None, ""): + end_time_raw = "0:0" + + # Format times with leading zeros (HH:MM) + start_time = self._format_time(start_time_raw) + end_time = self._format_time(end_time_raw) + + # Get battery mode + batt_mode_raw = self.data.get(f"forcedChargeBatMode{segment_id}", 0) + try: + batt_mode = int(batt_mode_raw) + except (ValueError, TypeError): + batt_mode = 0 + + mode_name = mode_names.get(batt_mode, "Unknown") + + # Get enabled status + enabled_raw = self.data.get(f"forcedChargeFlag{segment_id}", 0) + try: + enabled = bool(int(enabled_raw)) + except (ValueError, TypeError): + enabled = False + + return { + "segment_id": segment_id, + "start_time": start_time, + "end_time": end_time, + "batt_mode": batt_mode, + "mode_name": mode_name, + "enabled": enabled, + } + + def _format_time(self, time_raw: str) -> str: + """Format time string to HH:MM format.""" + try: + parts = str(time_raw).split(":") + hour = int(parts[0]) + minute = int(parts[1]) + except (ValueError, IndexError): + return "00:00" + else: + return f"{hour:02d}:{minute:02d}" diff --git a/homeassistant/components/growatt_server/icons.json b/homeassistant/components/growatt_server/icons.json new file mode 100644 index 00000000000000..61070b5b8e365d --- /dev/null +++ b/homeassistant/components/growatt_server/icons.json @@ -0,0 +1,10 @@ +{ + "services": { + "read_min_time_segments": { + "service": "mdi:clock-outline" + }, + "update_min_time_segment": { + "service": "mdi:clock-edit" + } + } +} diff --git a/homeassistant/components/growatt_server/services.yaml b/homeassistant/components/growatt_server/services.yaml new file mode 100644 index 00000000000000..d503a858d3d602 --- /dev/null +++ b/homeassistant/components/growatt_server/services.yaml @@ -0,0 +1,50 @@ +# Service definitions for Growatt Server integration +# Schemas are defined dynamically in code (__init__.py) using selectors + +update_min_time_segment: + fields: + segment_id: + required: true + example: 1 + selector: + number: + min: 1 + max: 9 + mode: box + batt_mode: + required: true + example: "load-first" + selector: + select: + options: + - "load-first" + - "battery-first" + - "grid-first" + start_time: + required: true + example: "08:00" + selector: + time: + end_time: + required: true + example: "12:00" + selector: + time: + enabled: + required: true + example: true + selector: + boolean: + device_id: + required: false + example: "MIN12345" + selector: + text: + +read_min_time_segments: + fields: + device_id: + required: false + example: "MIN12345" + selector: + text: diff --git a/homeassistant/components/growatt_server/strings.json b/homeassistant/components/growatt_server/strings.json index 4fc1b0658434cc..fcb912703531b0 100644 --- a/homeassistant/components/growatt_server/strings.json +++ b/homeassistant/components/growatt_server/strings.json @@ -42,20 +42,6 @@ } }, "entity": { - "number": { - "battery_charge_power_limit": { - "name": "Battery charge power limit" - }, - "battery_charge_soc_limit": { - "name": "Battery charge SOC limit" - }, - "battery_discharge_power_limit": { - "name": "Battery discharge power limit" - }, - "battery_discharge_soc_limit": { - "name": "Battery discharge SOC limit" - } - }, "sensor": { "inverter_amperage_input_1": { "name": "Input 1 amperage" @@ -523,5 +509,47 @@ } } }, + "services": { + "read_min_time_segments": { + "description": "Read all time-of-use (TOU) segments from a MIN inverter.", + "fields": { + "device_id": { + "description": "MIN device ID (only required when multiple MIN devices exist).", + "name": "Device ID" + } + }, + "name": "Read MIN time segments" + }, + "update_min_time_segment": { + "description": "Update a time-of-use (TOU) segment for MIN inverters.", + "fields": { + "batt_mode": { + "description": "Battery operation mode for this time segment.", + "name": "Battery mode" + }, + "device_id": { + "description": "MIN device ID (only required when multiple MIN devices exist).", + "name": "Device ID" + }, + "enabled": { + "description": "Whether this time segment is active.", + "name": "Enabled" + }, + "end_time": { + "description": "End time for the segment (HH:MM format).", + "name": "End time" + }, + "segment_id": { + "description": "Time segment ID (1-9).", + "name": "Segment ID" + }, + "start_time": { + "description": "Start time for the segment (HH:MM format).", + "name": "Start time" + } + }, + "name": "Update MIN time segment" + } + }, "title": "Growatt Server" } diff --git a/tests/components/growatt_server/conftest.py b/tests/components/growatt_server/conftest.py index ddee63e873d62c..c2a57c1b37406e 100644 --- a/tests/components/growatt_server/conftest.py +++ b/tests/components/growatt_server/conftest.py @@ -30,12 +30,15 @@ def mock_growatt_v1_api(): - plant_energy_overview: Called by total coordinator during first refresh Methods mocked for MIN device coordinator refresh: - - min_detail: Provides device state (e.g., acChargeEnable, chargePowerCommand) + - min_detail: Provides device state (e.g., acChargeEnable for switches) - min_settings: Provides settings (e.g. TOU periods) - - min_energy: Provides energy data (empty for switch/number tests, sensors need real data) + - min_energy: Provides energy data (empty for switch tests, sensors need real data) - Methods mocked for switch and number operations: - - min_write_parameter: Called by switch/number entities to change settings + Methods mocked for switch operations: + - min_write_parameter: Called by switch entities to change settings + + Methods mocked for service operations: + - min_write_time_segment: Called by time segment management services """ with patch("growattServer.OpenApiV1", autospec=True) as mock_v1_api_class: mock_v1_api = mock_v1_api_class.return_value @@ -54,19 +57,16 @@ def mock_growatt_v1_api(): mock_v1_api.min_detail.return_value = { "deviceSn": "MIN123456", "acChargeEnable": 1, # AC charge enabled - read by switch entity - "chargePowerCommand": 50, # 50% charge power - read by number entity - "wchargeSOCLowLimit": 10, # 10% charge stop SOC - read by number entity - "disChargePowerCommand": 80, # 80% discharge power - read by number entity - "wdisChargeSOCLowLimit": 20, # 20% discharge stop SOC - read by number entity } # Called by MIN device coordinator during refresh mock_v1_api.min_settings.return_value = { - # Forced charge time segments (not used by switch/number, but coordinator fetches it) + # Segment 1 - enabled "forcedTimeStart1": "06:00", "forcedTimeStop1": "08:00", "forcedChargeBatMode1": 1, "forcedChargeFlag1": 1, + # Segment 2 - disabled "forcedTimeStart2": "22:00", "forcedTimeStop2": "24:00", "forcedChargeBatMode2": 0, @@ -74,19 +74,8 @@ def mock_growatt_v1_api(): } # Called by MIN device coordinator during refresh - # Provide realistic energy data for sensor tests - mock_v1_api.min_energy.return_value = { - "eChargeToday": 5.2, - "eChargeTotal": 125.8, - "eDischargeToday": 8.1, - "eDischargeTotal": 245.6, - "eSelfToday": 12.5, - "eSelfTotal": 320.4, - "eBatChargeToday": 6.3, - "eBatChargeTotal": 150.2, - "eBatDischargeToday": 7.8, - "eBatDischargeTotal": 180.5, - } + # Empty dict is sufficient for switch tests (sensor tests would need real energy data) + mock_v1_api.min_energy.return_value = {} # Called by total coordinator during refresh mock_v1_api.plant_energy_overview.return_value = { @@ -95,9 +84,15 @@ def mock_growatt_v1_api(): "current_power": 2500, } - # Called by switch/number entities during turn_on/turn_off/set_value + # Called by switch entities during turn_on/turn_off mock_v1_api.min_write_parameter.return_value = None + # Called by time segment management services + mock_v1_api.min_write_time_segment.return_value = { + "error_code": 0, + "error_msg": "Success", + } + yield mock_v1_api @@ -170,7 +165,7 @@ def mock_config_entry() -> MockConfigEntry: CONF_PLANT_ID: "plant_123", "name": "Test Plant", }, - unique_id="plant_123", + unique_id="12345", ) diff --git a/tests/components/growatt_server/test_services.py b/tests/components/growatt_server/test_services.py new file mode 100644 index 00000000000000..a6741f6217e7d9 --- /dev/null +++ b/tests/components/growatt_server/test_services.py @@ -0,0 +1,395 @@ +"""Test Growatt Server services.""" + +from unittest.mock import patch + +import pytest +import voluptuous as vol + +from homeassistant.components.growatt_server.const import DOMAIN +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import HomeAssistantError + +from tests.common import MockConfigEntry + + +async def test_read_min_time_segments_single_device( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_api, +) -> None: + """Test reading MIN time segments for single device.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Test service call + response = await hass.services.async_call( + DOMAIN, + "read_min_time_segments", + {}, + blocking=True, + return_response=True, + ) + + assert response is not None + assert "time_segments" in response + assert len(response["time_segments"]) == 9 # Returns all 9 segments (1-9) + + +async def test_update_min_time_segment_charge_mode( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_api, +) -> None: + """Test updating MIN time segment with charge mode.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Test successful update + await hass.services.async_call( + DOMAIN, + "update_min_time_segment", + { + "segment_id": 1, + "start_time": "09:00", + "end_time": "11:00", + "batt_mode": "load-first", + "enabled": True, + }, + blocking=True, + ) + + # Verify the API was called + mock_growatt_api.min_write_time_segment.assert_called_once() + + +async def test_update_min_time_segment_discharge_mode( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_api, +) -> None: + """Test updating MIN time segment with discharge mode.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + await hass.services.async_call( + DOMAIN, + "update_min_time_segment", + { + "segment_id": 2, + "start_time": "14:00", + "end_time": "16:00", + "batt_mode": "battery-first", + "enabled": True, + }, + blocking=True, + ) + + mock_growatt_api.min_write_time_segment.assert_called_once() + + +async def test_update_min_time_segment_standby_mode( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_api, +) -> None: + """Test updating MIN time segment with standby mode.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + await hass.services.async_call( + DOMAIN, + "update_min_time_segment", + { + "segment_id": 3, + "start_time": "20:00", + "end_time": "22:00", + "batt_mode": "grid-first", + "enabled": True, + }, + blocking=True, + ) + + mock_growatt_api.min_write_time_segment.assert_called_once() + + +async def test_update_min_time_segment_disabled( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_api, +) -> None: + """Test disabling a MIN time segment.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + await hass.services.async_call( + DOMAIN, + "update_min_time_segment", + { + "segment_id": 1, + "start_time": "06:00", + "end_time": "08:00", + "batt_mode": "load-first", + "enabled": False, + }, + blocking=True, + ) + + mock_growatt_api.min_write_time_segment.assert_called_once() + + +async def test_update_min_time_segment_api_error( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_api, +) -> None: + """Test handling API error when updating MIN time segment.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Mock API error response + mock_growatt_api.min_write_time_segment.return_value = { + "error_code": 1, + "error_msg": "API Error", + } + + with pytest.raises(HomeAssistantError, match="Error updating MIN time segment"): + await hass.services.async_call( + DOMAIN, + "update_min_time_segment", + { + "segment_id": 1, + "start_time": "09:00", + "end_time": "11:00", + "batt_mode": "load-first", + "enabled": True, + }, + blocking=True, + ) + + +async def test_no_min_devices_skips_service_registration( + hass: HomeAssistant, + mock_config_entry_classic: MockConfigEntry, + mock_growatt_api, +) -> None: + """Test that no services are registered when no MIN devices exist.""" + mock_config_entry_classic.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + # Only non-MIN devices (TLX with classic API) + mock_get_devices.return_value = ( + [{"deviceSn": "TLX123456", "deviceType": "tlx"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry_classic.entry_id) + await hass.async_block_till_done() + + # Verify services are not registered + assert not hass.services.has_service(DOMAIN, "update_min_time_segment") + assert not hass.services.has_service(DOMAIN, "read_min_time_segments") + + +async def test_multiple_devices_require_device_id_in_schema( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_api, +) -> None: + """Test that multiple devices require device_id parameter in schema.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [ + {"deviceSn": "MIN123456", "deviceType": "min"}, + {"deviceSn": "MIN789012", "deviceType": "min"}, + ], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Verify services are registered + assert hass.services.has_service(DOMAIN, "update_min_time_segment") + assert hass.services.has_service(DOMAIN, "read_min_time_segments") + + # Test that schema validation requires device_id for update service + with pytest.raises(vol.MultipleInvalid, match="required key not provided"): + await hass.services.async_call( + DOMAIN, + "update_min_time_segment", + { + # Missing required device_id + "segment_id": 1, + "start_time": "09:00", + "end_time": "11:00", + "batt_mode": "load-first", + "enabled": True, + }, + blocking=True, + ) + + # Test that schema validation requires device_id for read service + with pytest.raises(vol.MultipleInvalid, match="required key not provided"): + await hass.services.async_call( + DOMAIN, + "read_min_time_segments", + {}, # Missing required device_id + blocking=True, + return_response=True, + ) + + +async def test_single_device_does_not_require_device_id( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_api, +) -> None: + """Test that single device works without device_id parameter.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Test update service works without device_id (single device) + await hass.services.async_call( + DOMAIN, + "update_min_time_segment", + { + "segment_id": 1, + "start_time": "09:00", + "end_time": "11:00", + "batt_mode": "load-first", + "enabled": True, + }, + blocking=True, + ) + + mock_growatt_api.min_write_time_segment.assert_called() + + # Test read service works without device_id (single device) + response = await hass.services.async_call( + DOMAIN, + "read_min_time_segments", + {}, + blocking=True, + return_response=True, + ) + + assert response is not None + assert "time_segments" in response + + +async def test_multiple_devices_with_valid_device_id_works( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_api, +) -> None: + """Test that multiple devices work when device_id is specified.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [ + {"deviceSn": "MIN123456", "deviceType": "min"}, + {"deviceSn": "MIN789012", "deviceType": "min"}, + ], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Test update service with specific device_id + await hass.services.async_call( + DOMAIN, + "update_min_time_segment", + { + "device_id": "MIN123456", + "segment_id": 1, + "start_time": "09:00", + "end_time": "11:00", + "batt_mode": "load-first", + "enabled": True, + }, + blocking=True, + ) + + mock_growatt_api.min_write_time_segment.assert_called_once() + + # Test read service with specific device_id + response = await hass.services.async_call( + DOMAIN, + "read_min_time_segments", + {"device_id": "MIN123456"}, + blocking=True, + return_response=True, + ) + + assert response is not None + assert "time_segments" in response From d6fea836b5ba6453a79f659e18f36d5486ca6053 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Mon, 13 Oct 2025 19:37:06 +0000 Subject: [PATCH 02/17] Fix service tests after rebase - Update fixture names from mock_growatt_api to mock_growatt_v1_api/mock_growatt_classic_api - Fix min_write_time_segment mock to accept variable arguments - Use correct Classic API fixture for non-MIN device test --- tests/components/growatt_server/conftest.py | 13 ++++--- .../growatt_server/test_services.py | 34 +++++++++---------- 2 files changed, 25 insertions(+), 22 deletions(-) diff --git a/tests/components/growatt_server/conftest.py b/tests/components/growatt_server/conftest.py index c2a57c1b37406e..c14662dc5950da 100644 --- a/tests/components/growatt_server/conftest.py +++ b/tests/components/growatt_server/conftest.py @@ -1,6 +1,6 @@ """Common fixtures for the Growatt server tests.""" -from unittest.mock import patch +from unittest.mock import Mock, patch import pytest @@ -88,10 +88,13 @@ def mock_growatt_v1_api(): mock_v1_api.min_write_parameter.return_value = None # Called by time segment management services - mock_v1_api.min_write_time_segment.return_value = { - "error_code": 0, - "error_msg": "Success", - } + # Note: Don't use autospec for this method as it needs to accept variable arguments + mock_v1_api.min_write_time_segment = Mock( + return_value={ + "error_code": 0, + "error_msg": "Success", + } + ) yield mock_v1_api diff --git a/tests/components/growatt_server/test_services.py b/tests/components/growatt_server/test_services.py index a6741f6217e7d9..659ca0b1f7cba0 100644 --- a/tests/components/growatt_server/test_services.py +++ b/tests/components/growatt_server/test_services.py @@ -15,7 +15,7 @@ async def test_read_min_time_segments_single_device( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_api, + mock_growatt_v1_api, ) -> None: """Test reading MIN time segments for single device.""" mock_config_entry.add_to_hass(hass) @@ -47,7 +47,7 @@ async def test_read_min_time_segments_single_device( async def test_update_min_time_segment_charge_mode( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_api, + mock_growatt_v1_api, ) -> None: """Test updating MIN time segment with charge mode.""" mock_config_entry.add_to_hass(hass) @@ -77,13 +77,13 @@ async def test_update_min_time_segment_charge_mode( ) # Verify the API was called - mock_growatt_api.min_write_time_segment.assert_called_once() + mock_growatt_v1_api.min_write_time_segment.assert_called_once() async def test_update_min_time_segment_discharge_mode( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_api, + mock_growatt_v1_api, ) -> None: """Test updating MIN time segment with discharge mode.""" mock_config_entry.add_to_hass(hass) @@ -111,13 +111,13 @@ async def test_update_min_time_segment_discharge_mode( blocking=True, ) - mock_growatt_api.min_write_time_segment.assert_called_once() + mock_growatt_v1_api.min_write_time_segment.assert_called_once() async def test_update_min_time_segment_standby_mode( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_api, + mock_growatt_v1_api, ) -> None: """Test updating MIN time segment with standby mode.""" mock_config_entry.add_to_hass(hass) @@ -145,13 +145,13 @@ async def test_update_min_time_segment_standby_mode( blocking=True, ) - mock_growatt_api.min_write_time_segment.assert_called_once() + mock_growatt_v1_api.min_write_time_segment.assert_called_once() async def test_update_min_time_segment_disabled( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_api, + mock_growatt_v1_api, ) -> None: """Test disabling a MIN time segment.""" mock_config_entry.add_to_hass(hass) @@ -179,13 +179,13 @@ async def test_update_min_time_segment_disabled( blocking=True, ) - mock_growatt_api.min_write_time_segment.assert_called_once() + mock_growatt_v1_api.min_write_time_segment.assert_called_once() async def test_update_min_time_segment_api_error( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_api, + mock_growatt_v1_api, ) -> None: """Test handling API error when updating MIN time segment.""" mock_config_entry.add_to_hass(hass) @@ -201,7 +201,7 @@ async def test_update_min_time_segment_api_error( await hass.async_block_till_done() # Mock API error response - mock_growatt_api.min_write_time_segment.return_value = { + mock_growatt_v1_api.min_write_time_segment.return_value = { "error_code": 1, "error_msg": "API Error", } @@ -224,7 +224,7 @@ async def test_update_min_time_segment_api_error( async def test_no_min_devices_skips_service_registration( hass: HomeAssistant, mock_config_entry_classic: MockConfigEntry, - mock_growatt_api, + mock_growatt_classic_api, ) -> None: """Test that no services are registered when no MIN devices exist.""" mock_config_entry_classic.add_to_hass(hass) @@ -248,7 +248,7 @@ async def test_no_min_devices_skips_service_registration( async def test_multiple_devices_require_device_id_in_schema( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_api, + mock_growatt_v1_api, ) -> None: """Test that multiple devices require device_id parameter in schema.""" mock_config_entry.add_to_hass(hass) @@ -300,7 +300,7 @@ async def test_multiple_devices_require_device_id_in_schema( async def test_single_device_does_not_require_device_id( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_api, + mock_growatt_v1_api, ) -> None: """Test that single device works without device_id parameter.""" mock_config_entry.add_to_hass(hass) @@ -329,7 +329,7 @@ async def test_single_device_does_not_require_device_id( blocking=True, ) - mock_growatt_api.min_write_time_segment.assert_called() + mock_growatt_v1_api.min_write_time_segment.assert_called() # Test read service works without device_id (single device) response = await hass.services.async_call( @@ -347,7 +347,7 @@ async def test_single_device_does_not_require_device_id( async def test_multiple_devices_with_valid_device_id_works( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_api, + mock_growatt_v1_api, ) -> None: """Test that multiple devices work when device_id is specified.""" mock_config_entry.add_to_hass(hass) @@ -380,7 +380,7 @@ async def test_multiple_devices_with_valid_device_id_works( blocking=True, ) - mock_growatt_api.min_write_time_segment.assert_called_once() + mock_growatt_v1_api.min_write_time_segment.assert_called_once() # Test read service with specific device_id response = await hass.services.async_call( From 9ba44b570ceace66231f077058bbe32d9492b791 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Tue, 14 Oct 2025 05:40:51 +0000 Subject: [PATCH 03/17] Generalize service names for time segment management Rename services from update_min_time_segment/read_min_time_segments to update_time_segment/read_time_segments to support future expansion to other inverter types beyond MIN devices. --- .../components/growatt_server/__init__.py | 30 +++++----- .../components/growatt_server/coordinator.py | 18 +++--- .../components/growatt_server/icons.json | 4 +- .../components/growatt_server/services.yaml | 4 +- .../components/growatt_server/strings.json | 16 ++--- .../growatt_server/test_services.py | 58 +++++++++---------- 6 files changed, 65 insertions(+), 65 deletions(-) diff --git a/homeassistant/components/growatt_server/__init__.py b/homeassistant/components/growatt_server/__init__.py index 011b9c8ab47a1c..84c04d144f8eb1 100644 --- a/homeassistant/components/growatt_server/__init__.py +++ b/homeassistant/components/growatt_server/__init__.py @@ -244,8 +244,8 @@ def get_coordinator(device_id: str | None = None) -> GrowattCoordinator: return min_coordinators[device_id] - async def handle_update_min_time_segment(call: ServiceCall) -> None: - """Handle update_min_time_segment service call.""" + async def handle_update_time_segment(call: ServiceCall) -> None: + """Handle update_time_segment service call.""" segment_id = call.data["segment_id"] batt_mode_str = str(call.data["batt_mode"]) start_time_str = call.data["start_time"] @@ -254,7 +254,7 @@ async def handle_update_min_time_segment(call: ServiceCall) -> None: device_id = call.data.get("device_id") _LOGGER.debug( - "handle_update_min_time_segment: segment_id=%d, batt_mode=%s, start=%s, end=%s, enabled=%s, device_id=%s", + "handle_update_time_segment: segment_id=%d, batt_mode=%s, start=%s, end=%s, enabled=%s, device_id=%s", segment_id, batt_mode_str, start_time_str, @@ -283,7 +283,7 @@ async def handle_update_min_time_segment(call: ServiceCall) -> None: coordinator = get_coordinator(device_id) try: - await coordinator.update_min_time_segment( + await coordinator.update_time_segment( segment_id, batt_mode, start_time, @@ -292,24 +292,24 @@ async def handle_update_min_time_segment(call: ServiceCall) -> None: ) except Exception as err: _LOGGER.error( - "Error updating MIN time segment %d: %s", + "Error updating time segment %d: %s", segment_id, err, ) raise HomeAssistantError( - f"Error updating MIN time segment {segment_id}: {err}" + f"Error updating time segment {segment_id}: {err}" ) from err - async def handle_read_min_time_segments(call: ServiceCall) -> dict: - """Handle read_min_time_segments service call.""" + async def handle_read_time_segments(call: ServiceCall) -> dict: + """Handle read_time_segments service call.""" # Get the appropriate MIN coordinator coordinator = get_coordinator(call.data.get("device_id")) try: - time_segments = await coordinator.read_min_time_segments() + time_segments = await coordinator.read_time_segments() except Exception as err: - _LOGGER.error("Error reading MIN time segments: %s", err) - raise HomeAssistantError(f"Error reading MIN time segments: {err}") from err + _LOGGER.error("Error reading time segments: %s", err) + raise HomeAssistantError(f"Error reading time segments: {err}") from err else: return {"time_segments": time_segments} @@ -353,11 +353,11 @@ async def handle_read_min_time_segments(call: ServiceCall) -> dict: # Register services services_to_register = [ ( - "update_min_time_segment", - handle_update_min_time_segment, + "update_time_segment", + handle_update_time_segment, update_schema_fields, ), - ("read_min_time_segments", handle_read_min_time_segments, read_schema_fields), + ("read_time_segments", handle_read_time_segments, read_schema_fields), ] for service_name, handler, schema_fields in services_to_register: @@ -365,7 +365,7 @@ async def handle_read_min_time_segments(call: ServiceCall) -> dict: schema = vol.Schema(schema_fields) if schema_fields else None supports_response = ( SupportsResponse.ONLY - if service_name == "read_min_time_segments" + if service_name == "read_time_segments" else SupportsResponse.NONE ) diff --git a/homeassistant/components/growatt_server/coordinator.py b/homeassistant/components/growatt_server/coordinator.py index e749dd2f108ede..08431c3c0b0667 100644 --- a/homeassistant/components/growatt_server/coordinator.py +++ b/homeassistant/components/growatt_server/coordinator.py @@ -253,10 +253,10 @@ def get_data( return return_value - async def update_min_time_segment( + async def update_time_segment( self, segment_id: int, batt_mode: int, start_time, end_time, enabled: bool ) -> None: - """Update a MIN inverter time segment. + """Update an inverter time segment. Args: segment_id: Time segment ID (1-9) @@ -266,7 +266,7 @@ async def update_min_time_segment( enabled: Whether the segment is enabled """ _LOGGER.debug( - "Updating MIN time segment %s for device %s", + "Updating time segment %s for device %s", segment_id, self.device_id, ) @@ -292,7 +292,7 @@ async def update_min_time_segment( ) except growattServer.GrowattV1ApiError as err: _LOGGER.error( - "API error updating MIN time segment %s for device %s: %s", + "API error updating time segment %s for device %s: %s", segment_id, self.device_id, err, @@ -303,7 +303,7 @@ async def update_min_time_segment( if response.get("error_code", 1) != 0: error_msg = response.get("error_msg", "Unknown error") _LOGGER.error( - "Failed to update MIN time segment %s for device %s: %s", + "Failed to update time segment %s for device %s: %s", segment_id, self.device_id, error_msg, @@ -311,21 +311,21 @@ async def update_min_time_segment( raise HomeAssistantError(f"Failed to update time segment: {error_msg}") _LOGGER.info( - "Successfully updated MIN time segment %s for device %s", + "Successfully updated time segment %s for device %s", segment_id, self.device_id, ) # Trigger a refresh to update the data await self.async_refresh() - async def read_min_time_segments(self) -> list[dict]: - """Read time segments from a MIN inverter. + async def read_time_segments(self) -> list[dict]: + """Read time segments from an inverter. Returns: List of dictionaries containing segment information """ _LOGGER.debug( - "Reading MIN time segments for device %s", + "Reading time segments for device %s", self.device_id, ) diff --git a/homeassistant/components/growatt_server/icons.json b/homeassistant/components/growatt_server/icons.json index 61070b5b8e365d..091ab64276079f 100644 --- a/homeassistant/components/growatt_server/icons.json +++ b/homeassistant/components/growatt_server/icons.json @@ -1,9 +1,9 @@ { "services": { - "read_min_time_segments": { + "read_time_segments": { "service": "mdi:clock-outline" }, - "update_min_time_segment": { + "update_time_segment": { "service": "mdi:clock-edit" } } diff --git a/homeassistant/components/growatt_server/services.yaml b/homeassistant/components/growatt_server/services.yaml index d503a858d3d602..b18c792bbabd4f 100644 --- a/homeassistant/components/growatt_server/services.yaml +++ b/homeassistant/components/growatt_server/services.yaml @@ -1,7 +1,7 @@ # Service definitions for Growatt Server integration # Schemas are defined dynamically in code (__init__.py) using selectors -update_min_time_segment: +update_time_segment: fields: segment_id: required: true @@ -41,7 +41,7 @@ update_min_time_segment: selector: text: -read_min_time_segments: +read_time_segments: fields: device_id: required: false diff --git a/homeassistant/components/growatt_server/strings.json b/homeassistant/components/growatt_server/strings.json index fcb912703531b0..d5f1a069820304 100644 --- a/homeassistant/components/growatt_server/strings.json +++ b/homeassistant/components/growatt_server/strings.json @@ -510,25 +510,25 @@ } }, "services": { - "read_min_time_segments": { - "description": "Read all time-of-use (TOU) segments from a MIN inverter.", + "read_time_segments": { + "description": "Read all time-of-use (TOU) segments from a supported inverter.", "fields": { "device_id": { - "description": "MIN device ID (only required when multiple MIN devices exist).", + "description": "Device ID (only required when multiple devices exist).", "name": "Device ID" } }, - "name": "Read MIN time segments" + "name": "Read time segments" }, - "update_min_time_segment": { - "description": "Update a time-of-use (TOU) segment for MIN inverters.", + "update_time_segment": { + "description": "Update a time-of-use (TOU) segment for supported inverters.", "fields": { "batt_mode": { "description": "Battery operation mode for this time segment.", "name": "Battery mode" }, "device_id": { - "description": "MIN device ID (only required when multiple MIN devices exist).", + "description": "Device ID (only required when multiple devices exist).", "name": "Device ID" }, "enabled": { @@ -548,7 +548,7 @@ "name": "Start time" } }, - "name": "Update MIN time segment" + "name": "Update time segment" } }, "title": "Growatt Server" diff --git a/tests/components/growatt_server/test_services.py b/tests/components/growatt_server/test_services.py index 659ca0b1f7cba0..0f38b42c4434be 100644 --- a/tests/components/growatt_server/test_services.py +++ b/tests/components/growatt_server/test_services.py @@ -12,12 +12,12 @@ from tests.common import MockConfigEntry -async def test_read_min_time_segments_single_device( +async def test_read_time_segments_single_device( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, ) -> None: - """Test reading MIN time segments for single device.""" + """Test reading time segments for single device.""" mock_config_entry.add_to_hass(hass) with patch( @@ -33,7 +33,7 @@ async def test_read_min_time_segments_single_device( # Test service call response = await hass.services.async_call( DOMAIN, - "read_min_time_segments", + "read_time_segments", {}, blocking=True, return_response=True, @@ -44,12 +44,12 @@ async def test_read_min_time_segments_single_device( assert len(response["time_segments"]) == 9 # Returns all 9 segments (1-9) -async def test_update_min_time_segment_charge_mode( +async def test_update_time_segment_charge_mode( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, ) -> None: - """Test updating MIN time segment with charge mode.""" + """Test updating time segment with charge mode.""" mock_config_entry.add_to_hass(hass) with patch( @@ -65,7 +65,7 @@ async def test_update_min_time_segment_charge_mode( # Test successful update await hass.services.async_call( DOMAIN, - "update_min_time_segment", + "update_time_segment", { "segment_id": 1, "start_time": "09:00", @@ -80,12 +80,12 @@ async def test_update_min_time_segment_charge_mode( mock_growatt_v1_api.min_write_time_segment.assert_called_once() -async def test_update_min_time_segment_discharge_mode( +async def test_update_time_segment_discharge_mode( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, ) -> None: - """Test updating MIN time segment with discharge mode.""" + """Test updating time segment with discharge mode.""" mock_config_entry.add_to_hass(hass) with patch( @@ -100,7 +100,7 @@ async def test_update_min_time_segment_discharge_mode( await hass.services.async_call( DOMAIN, - "update_min_time_segment", + "update_time_segment", { "segment_id": 2, "start_time": "14:00", @@ -114,12 +114,12 @@ async def test_update_min_time_segment_discharge_mode( mock_growatt_v1_api.min_write_time_segment.assert_called_once() -async def test_update_min_time_segment_standby_mode( +async def test_update_time_segment_standby_mode( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, ) -> None: - """Test updating MIN time segment with standby mode.""" + """Test updating time segment with standby mode.""" mock_config_entry.add_to_hass(hass) with patch( @@ -134,7 +134,7 @@ async def test_update_min_time_segment_standby_mode( await hass.services.async_call( DOMAIN, - "update_min_time_segment", + "update_time_segment", { "segment_id": 3, "start_time": "20:00", @@ -148,12 +148,12 @@ async def test_update_min_time_segment_standby_mode( mock_growatt_v1_api.min_write_time_segment.assert_called_once() -async def test_update_min_time_segment_disabled( +async def test_update_time_segment_disabled( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, ) -> None: - """Test disabling a MIN time segment.""" + """Test disabling a time segment.""" mock_config_entry.add_to_hass(hass) with patch( @@ -168,7 +168,7 @@ async def test_update_min_time_segment_disabled( await hass.services.async_call( DOMAIN, - "update_min_time_segment", + "update_time_segment", { "segment_id": 1, "start_time": "06:00", @@ -182,12 +182,12 @@ async def test_update_min_time_segment_disabled( mock_growatt_v1_api.min_write_time_segment.assert_called_once() -async def test_update_min_time_segment_api_error( +async def test_update_time_segment_api_error( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, ) -> None: - """Test handling API error when updating MIN time segment.""" + """Test handling API error when updating time segment.""" mock_config_entry.add_to_hass(hass) with patch( @@ -206,10 +206,10 @@ async def test_update_min_time_segment_api_error( "error_msg": "API Error", } - with pytest.raises(HomeAssistantError, match="Error updating MIN time segment"): + with pytest.raises(HomeAssistantError, match="Error updating time segment"): await hass.services.async_call( DOMAIN, - "update_min_time_segment", + "update_time_segment", { "segment_id": 1, "start_time": "09:00", @@ -241,8 +241,8 @@ async def test_no_min_devices_skips_service_registration( await hass.async_block_till_done() # Verify services are not registered - assert not hass.services.has_service(DOMAIN, "update_min_time_segment") - assert not hass.services.has_service(DOMAIN, "read_min_time_segments") + assert not hass.services.has_service(DOMAIN, "update_time_segment") + assert not hass.services.has_service(DOMAIN, "read_time_segments") async def test_multiple_devices_require_device_id_in_schema( @@ -267,14 +267,14 @@ async def test_multiple_devices_require_device_id_in_schema( await hass.async_block_till_done() # Verify services are registered - assert hass.services.has_service(DOMAIN, "update_min_time_segment") - assert hass.services.has_service(DOMAIN, "read_min_time_segments") + assert hass.services.has_service(DOMAIN, "update_time_segment") + assert hass.services.has_service(DOMAIN, "read_time_segments") # Test that schema validation requires device_id for update service with pytest.raises(vol.MultipleInvalid, match="required key not provided"): await hass.services.async_call( DOMAIN, - "update_min_time_segment", + "update_time_segment", { # Missing required device_id "segment_id": 1, @@ -290,7 +290,7 @@ async def test_multiple_devices_require_device_id_in_schema( with pytest.raises(vol.MultipleInvalid, match="required key not provided"): await hass.services.async_call( DOMAIN, - "read_min_time_segments", + "read_time_segments", {}, # Missing required device_id blocking=True, return_response=True, @@ -318,7 +318,7 @@ async def test_single_device_does_not_require_device_id( # Test update service works without device_id (single device) await hass.services.async_call( DOMAIN, - "update_min_time_segment", + "update_time_segment", { "segment_id": 1, "start_time": "09:00", @@ -334,7 +334,7 @@ async def test_single_device_does_not_require_device_id( # Test read service works without device_id (single device) response = await hass.services.async_call( DOMAIN, - "read_min_time_segments", + "read_time_segments", {}, blocking=True, return_response=True, @@ -368,7 +368,7 @@ async def test_multiple_devices_with_valid_device_id_works( # Test update service with specific device_id await hass.services.async_call( DOMAIN, - "update_min_time_segment", + "update_time_segment", { "device_id": "MIN123456", "segment_id": 1, @@ -385,7 +385,7 @@ async def test_multiple_devices_with_valid_device_id_works( # Test read service with specific device_id response = await hass.services.async_call( DOMAIN, - "read_min_time_segments", + "read_time_segments", {"device_id": "MIN123456"}, blocking=True, return_response=True, From 31b2432e6d57bc88590a9da4346d4607a9ab1258 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Fri, 17 Oct 2025 13:39:25 +0000 Subject: [PATCH 04/17] Improve service code quality and test coverage - Fix bare Exception catches in service handlers, replace with specific API exceptions - Remove debug logging leftover from development - Add comprehensive error handling tests for services - Improve test coverage for service error paths - All 42 tests passing with 85% overall coverage --- .../components/growatt_server/__init__.py | 6 +- .../growatt_server/test_services.py | 69 +++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/growatt_server/__init__.py b/homeassistant/components/growatt_server/__init__.py index 84c04d144f8eb1..a9728472a33270 100644 --- a/homeassistant/components/growatt_server/__init__.py +++ b/homeassistant/components/growatt_server/__init__.py @@ -44,9 +44,7 @@ def get_device_list_classic( # Log in to api and fetch first plant if no plant id is defined. try: login_response = api.login(config[CONF_USERNAME], config[CONF_PASSWORD]) - # DEBUG: Log the actual response structure except Exception as ex: - _LOGGER.error("DEBUG - Login response: %s", login_response) raise ConfigEntryError( f"Error communicating with Growatt API during login: {ex}" ) from ex @@ -290,7 +288,7 @@ async def handle_update_time_segment(call: ServiceCall) -> None: end_time, enabled, ) - except Exception as err: + except (growattServer.GrowattV1ApiError, HomeAssistantError) as err: _LOGGER.error( "Error updating time segment %d: %s", segment_id, @@ -307,7 +305,7 @@ async def handle_read_time_segments(call: ServiceCall) -> dict: try: time_segments = await coordinator.read_time_segments() - except Exception as err: + except (growattServer.GrowattV1ApiError, HomeAssistantError) as err: _LOGGER.error("Error reading time segments: %s", err) raise HomeAssistantError(f"Error reading time segments: {err}") from err else: diff --git a/tests/components/growatt_server/test_services.py b/tests/components/growatt_server/test_services.py index 0f38b42c4434be..3be99f611a6b2a 100644 --- a/tests/components/growatt_server/test_services.py +++ b/tests/components/growatt_server/test_services.py @@ -393,3 +393,72 @@ async def test_multiple_devices_with_valid_device_id_works( assert response is not None assert "time_segments" in response + + +async def test_update_time_segment_invalid_time_format( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_v1_api, +) -> None: + """Test handling invalid time format in update_time_segment.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Test with invalid time format - schema validates before handler sees it + with pytest.raises(vol.MultipleInvalid, match="Invalid time specified"): + await hass.services.async_call( + DOMAIN, + "update_time_segment", + { + "segment_id": 1, + "start_time": "invalid", + "end_time": "11:00", + "batt_mode": "load-first", + "enabled": True, + }, + blocking=True, + ) + + +async def test_read_time_segments_api_error( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_v1_api, +) -> None: + """Test handling API error when reading time segments.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Mock API error by making coordinator.read_time_segments raise an exception + with ( + patch( + "homeassistant.components.growatt_server.coordinator.GrowattCoordinator.read_time_segments", + side_effect=HomeAssistantError("API connection failed"), + ), + pytest.raises(HomeAssistantError, match="Error reading time segments"), + ): + await hass.services.async_call( + DOMAIN, + "read_time_segments", + {}, + blocking=True, + return_response=True, + ) From 2de6c9bd2314dd23eb414a66cc20096d62e4bf80 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Sat, 18 Oct 2025 20:32:19 +0000 Subject: [PATCH 05/17] Refactor test structure for improved readability and maintainability --- tests/components/growatt_server/test_services.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/components/growatt_server/test_services.py b/tests/components/growatt_server/test_services.py index 3be99f611a6b2a..495ac082c83876 100644 --- a/tests/components/growatt_server/test_services.py +++ b/tests/components/growatt_server/test_services.py @@ -2,6 +2,7 @@ from unittest.mock import patch +import growattServer import pytest import voluptuous as vol @@ -200,11 +201,14 @@ async def test_update_time_segment_api_error( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() - # Mock API error response - mock_growatt_v1_api.min_write_time_segment.return_value = { - "error_code": 1, - "error_msg": "API Error", - } + # Mock API error - the library raises an exception instead of returning error dict + mock_growatt_v1_api.min_write_time_segment.side_effect = ( + growattServer.GrowattV1ApiError( + "Error during writing time segment 1", + error_code=1, + error_msg="API Error", + ) + ) with pytest.raises(HomeAssistantError, match="Error updating time segment"): await hass.services.async_call( From 0425f6b95ed71c3855a8a3671c12982db0a1d1df Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Sat, 18 Oct 2025 20:37:45 +0000 Subject: [PATCH 06/17] Simplify return statement in _async_register_services function --- homeassistant/components/growatt_server/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/growatt_server/__init__.py b/homeassistant/components/growatt_server/__init__.py index a9728472a33270..41301c7fbd9f0a 100644 --- a/homeassistant/components/growatt_server/__init__.py +++ b/homeassistant/components/growatt_server/__init__.py @@ -308,8 +308,8 @@ async def handle_read_time_segments(call: ServiceCall) -> dict: except (growattServer.GrowattV1ApiError, HomeAssistantError) as err: _LOGGER.error("Error reading time segments: %s", err) raise HomeAssistantError(f"Error reading time segments: {err}") from err - else: - return {"time_segments": time_segments} + + return {"time_segments": time_segments} # Create device selector schema helper device_selector_fields = {} From a85fe48857b89029eb72e1b945e9debffe06cd82 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Sun, 26 Oct 2025 10:42:45 +0000 Subject: [PATCH 07/17] adress review comments --- .../components/growatt_server/__init__.py | 206 ++---------------- .../components/growatt_server/coordinator.py | 104 ++++----- .../components/growatt_server/services.py | 181 +++++++++++++++ .../components/growatt_server/services.yaml | 18 +- .../components/growatt_server/strings.json | 9 + .../growatt_server/test_services.py | 202 ++++++++++++++--- 6 files changed, 425 insertions(+), 295 deletions(-) create mode 100644 homeassistant/components/growatt_server/services.py diff --git a/homeassistant/components/growatt_server/__init__.py b/homeassistant/components/growatt_server/__init__.py index 41301c7fbd9f0a..93ede43b90dd1d 100644 --- a/homeassistant/components/growatt_server/__init__.py +++ b/homeassistant/components/growatt_server/__init__.py @@ -1,25 +1,19 @@ """The Growatt server PV inverter sensor integration.""" from collections.abc import Mapping -from datetime import datetime import logging import growattServer -import voluptuous as vol from homeassistant.const import CONF_PASSWORD, CONF_TOKEN, CONF_URL, CONF_USERNAME -from homeassistant.core import HomeAssistant, ServiceCall, SupportsResponse -from homeassistant.exceptions import ( - ConfigEntryAuthFailed, - ConfigEntryError, - HomeAssistantError, -) -from homeassistant.helpers import selector +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryError +from homeassistant.helpers import config_validation as cv +from homeassistant.helpers.typing import ConfigType from .const import ( AUTH_API_TOKEN, AUTH_PASSWORD, - BATT_MODE_MAP, CONF_AUTH_TYPE, CONF_PLANT_ID, DEFAULT_PLANT_ID, @@ -31,9 +25,19 @@ ) from .coordinator import GrowattConfigEntry, GrowattCoordinator from .models import GrowattRuntimeData +from .services import async_register_services _LOGGER = logging.getLogger(__name__) +CONFIG_SCHEMA = cv.config_entry_only_config_schema(DOMAIN) + + +async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool: + """Set up the Growatt Server component.""" + # Register services + await async_register_services(hass) + return True + def get_device_list_classic( api: growattServer.GrowattApi, config: Mapping[str, str] @@ -192,191 +196,9 @@ async def async_setup_entry( # Set up all the entities await hass.config_entries.async_forward_entry_setups(config_entry, PLATFORMS) - # Register services for MIN/TLX devices - await _async_register_services(hass, config_entry, device_coordinators) - return True -async def _async_register_services( - hass: HomeAssistant, - config_entry: GrowattConfigEntry, - device_coordinators: dict, -) -> None: - """Register services for MIN/TLX devices.""" - # Get all MIN coordinators with V1 API - single source of truth - min_coordinators = { - coord.device_id: coord - for coord in device_coordinators.values() - if coord.device_type == "min" and coord.api_version == "v1" - } - - if not min_coordinators: - _LOGGER.debug( - "No MIN devices with V1 API found, skipping TOU service registration. " - "Services require MIN devices with token authentication" - ) - return - - _LOGGER.info( - "Found %d MIN device(s) with V1 API, registering TOU services", - len(min_coordinators), - ) - - def get_coordinator(device_id: str | None = None) -> GrowattCoordinator: - """Get coordinator by device_id with consistent behavior.""" - if device_id is None: - if len(min_coordinators) == 1: - # Only one device - return it - return next(iter(min_coordinators.values())) - # Multiple devices - require explicit selection - device_list = ", ".join(min_coordinators.keys()) - raise HomeAssistantError( - f"Multiple MIN devices available ({device_list}). " - "Please specify device_id parameter." - ) - - # Explicit device_id provided - if device_id not in min_coordinators: - raise HomeAssistantError(f"MIN device '{device_id}' not found") - - return min_coordinators[device_id] - - async def handle_update_time_segment(call: ServiceCall) -> None: - """Handle update_time_segment service call.""" - segment_id = call.data["segment_id"] - batt_mode_str = str(call.data["batt_mode"]) - start_time_str = call.data["start_time"] - end_time_str = call.data["end_time"] - enabled = call.data["enabled"] - device_id = call.data.get("device_id") - - _LOGGER.debug( - "handle_update_time_segment: segment_id=%d, batt_mode=%s, start=%s, end=%s, enabled=%s, device_id=%s", - segment_id, - batt_mode_str, - start_time_str, - end_time_str, - enabled, - device_id, - ) - - # Convert batt_mode string to integer - batt_mode = BATT_MODE_MAP.get(batt_mode_str) - if batt_mode is None: - _LOGGER.error("Invalid battery mode: %s", batt_mode_str) - raise HomeAssistantError(f"Invalid battery mode: {batt_mode_str}") - - # Convert time strings to datetime.time objects - try: - start_time = datetime.strptime(start_time_str, "%H:%M").time() - end_time = datetime.strptime(end_time_str, "%H:%M").time() - except ValueError as err: - _LOGGER.error("Start_time and end_time must be in HH:MM format") - raise HomeAssistantError( - "start_time and end_time must be in HH:MM format" - ) from err - - # Get the appropriate MIN coordinator - coordinator = get_coordinator(device_id) - - try: - await coordinator.update_time_segment( - segment_id, - batt_mode, - start_time, - end_time, - enabled, - ) - except (growattServer.GrowattV1ApiError, HomeAssistantError) as err: - _LOGGER.error( - "Error updating time segment %d: %s", - segment_id, - err, - ) - raise HomeAssistantError( - f"Error updating time segment {segment_id}: {err}" - ) from err - - async def handle_read_time_segments(call: ServiceCall) -> dict: - """Handle read_time_segments service call.""" - # Get the appropriate MIN coordinator - coordinator = get_coordinator(call.data.get("device_id")) - - try: - time_segments = await coordinator.read_time_segments() - except (growattServer.GrowattV1ApiError, HomeAssistantError) as err: - _LOGGER.error("Error reading time segments: %s", err) - raise HomeAssistantError(f"Error reading time segments: {err}") from err - - return {"time_segments": time_segments} - - # Create device selector schema helper - device_selector_fields = {} - if len(min_coordinators) > 1: - device_options = [ - selector.SelectOptionDict(value=device_id, label=f"MIN Device {device_id}") - for device_id in min_coordinators - ] - device_selector_fields[vol.Required("device_id")] = selector.SelectSelector( - selector.SelectSelectorConfig(options=device_options) - ) - - # Define service schemas - update_schema_fields = { - vol.Required("segment_id"): selector.NumberSelector( - selector.NumberSelectorConfig( - min=1, max=9, mode=selector.NumberSelectorMode.BOX - ) - ), - vol.Required("batt_mode"): selector.SelectSelector( - selector.SelectSelectorConfig( - options=[ - selector.SelectOptionDict(value="load-first", label="Load First"), - selector.SelectOptionDict( - value="battery-first", label="Battery First" - ), - selector.SelectOptionDict(value="grid-first", label="Grid First"), - ] - ) - ), - vol.Required("start_time"): selector.TimeSelector(), - vol.Required("end_time"): selector.TimeSelector(), - vol.Required("enabled"): selector.BooleanSelector(), - **device_selector_fields, - } - - read_schema_fields = {**device_selector_fields} - - # Register services - services_to_register = [ - ( - "update_time_segment", - handle_update_time_segment, - update_schema_fields, - ), - ("read_time_segments", handle_read_time_segments, read_schema_fields), - ] - - for service_name, handler, schema_fields in services_to_register: - if not hass.services.has_service(DOMAIN, service_name): - schema = vol.Schema(schema_fields) if schema_fields else None - supports_response = ( - SupportsResponse.ONLY - if service_name == "read_time_segments" - else SupportsResponse.NONE - ) - - hass.services.async_register( - DOMAIN, - service_name, - handler, - schema=schema, - supports_response=supports_response, - ) - _LOGGER.info("Registered service: %s", service_name) - - async def async_unload_entry( hass: HomeAssistant, config_entry: GrowattConfigEntry ) -> bool: diff --git a/homeassistant/components/growatt_server/coordinator.py b/homeassistant/components/growatt_server/coordinator.py index 08431c3c0b0667..22f8bfd7709848 100644 --- a/homeassistant/components/growatt_server/coordinator.py +++ b/homeassistant/components/growatt_server/coordinator.py @@ -12,11 +12,17 @@ from homeassistant.config_entries import ConfigEntry from homeassistant.const import CONF_PASSWORD, CONF_URL, CONF_USERNAME from homeassistant.core import HomeAssistant -from homeassistant.exceptions import HomeAssistantError +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed from homeassistant.util import dt as dt_util -from .const import DEFAULT_URL, DOMAIN +from .const import ( + BATT_MODE_BATTERY_FIRST, + BATT_MODE_GRID_FIRST, + BATT_MODE_LOAD_FIRST, + DEFAULT_URL, + DOMAIN, +) from .models import GrowattRuntimeData if TYPE_CHECKING: @@ -266,22 +272,24 @@ async def update_time_segment( enabled: Whether the segment is enabled """ _LOGGER.debug( - "Updating time segment %s for device %s", + "Updating time segment %d for device %s (mode=%d, %s-%s, enabled=%s)", segment_id, self.device_id, + batt_mode, + start_time, + end_time, + enabled, ) if self.api_version != "v1": - _LOGGER.warning( - "Updating time segments is only supported with V1 API (token authentication)" - ) - raise HomeAssistantError( + raise ServiceValidationError( "Updating time segments requires token authentication" ) try: # Use V1 API for token authentication - response = await self.hass.async_add_executor_job( + # The library's _process_response will raise GrowattV1ApiError if error_code != 0 + await self.hass.async_add_executor_job( self.api.min_write_time_segment, self.device_id, segment_id, @@ -291,108 +299,72 @@ async def update_time_segment( enabled, ) except growattServer.GrowattV1ApiError as err: - _LOGGER.error( - "API error updating time segment %s for device %s: %s", - segment_id, - self.device_id, - err, - ) raise HomeAssistantError(f"API error updating time segment: {err}") from err - # Check response error code - if response.get("error_code", 1) != 0: - error_msg = response.get("error_msg", "Unknown error") - _LOGGER.error( - "Failed to update time segment %s for device %s: %s", - segment_id, - self.device_id, - error_msg, - ) - raise HomeAssistantError(f"Failed to update time segment: {error_msg}") - - _LOGGER.info( - "Successfully updated time segment %s for device %s", - segment_id, - self.device_id, - ) - # Trigger a refresh to update the data - await self.async_refresh() - async def read_time_segments(self) -> list[dict]: """Read time segments from an inverter. Returns: List of dictionaries containing segment information """ - _LOGGER.debug( - "Reading time segments for device %s", - self.device_id, - ) + _LOGGER.debug("Reading time segments for device %s", self.device_id) if self.api_version != "v1": - _LOGGER.warning( - "Reading time segments is only supported with V1 API (token authentication)" - ) - raise HomeAssistantError( + raise ServiceValidationError( "Reading time segments requires token authentication" ) # Ensure we have current data if not self.data: - _LOGGER.debug("Triggering refresh to get time segments") + _LOGGER.debug("Coordinator data not available, triggering refresh") await self.async_refresh() time_segments = [] - mode_names = {0: "Load First", 1: "Battery First", 2: "Grid First"} # Extract time segments from coordinator data for i in range(1, 10): # Segments 1-9 - segment = self._parse_time_segment(i, mode_names) + segment = self._parse_time_segment(i) time_segments.append(segment) - _LOGGER.debug( - "Read %d time segments for device %s", len(time_segments), self.device_id - ) return time_segments - def _parse_time_segment(self, segment_id: int, mode_names: dict[int, str]) -> dict: + def _parse_time_segment(self, segment_id: int) -> dict: """Parse a single time segment from coordinator data.""" - # Get raw time values - start_time_raw = self.data.get(f"forcedTimeStart{segment_id}", "0:0") - end_time_raw = self.data.get(f"forcedTimeStop{segment_id}", "0:0") + # Get raw time values - these should always be present from the API + start_time_raw = self.data.get(f"forcedTimeStart{segment_id}") + end_time_raw = self.data.get(f"forcedTimeStop{segment_id}") - # Handle 'null' or empty values + # Handle 'null' or empty values from API if start_time_raw in ("null", None, ""): start_time_raw = "0:0" if end_time_raw in ("null", None, ""): end_time_raw = "0:0" # Format times with leading zeros (HH:MM) - start_time = self._format_time(start_time_raw) - end_time = self._format_time(end_time_raw) + start_time = self._format_time(str(start_time_raw)) + end_time = self._format_time(str(end_time_raw)) # Get battery mode - batt_mode_raw = self.data.get(f"forcedChargeBatMode{segment_id}", 0) - try: - batt_mode = int(batt_mode_raw) - except (ValueError, TypeError): - batt_mode = 0 + batt_mode_int = int( + self.data.get(f"forcedChargeBatMode{segment_id}", BATT_MODE_LOAD_FIRST) + ) - mode_name = mode_names.get(batt_mode, "Unknown") + # Map numeric mode to string key (matches update_time_segment input format) + mode_map = { + BATT_MODE_LOAD_FIRST: "load_first", + BATT_MODE_BATTERY_FIRST: "battery_first", + BATT_MODE_GRID_FIRST: "grid_first", + } + batt_mode = mode_map.get(batt_mode_int, "load_first") # Get enabled status - enabled_raw = self.data.get(f"forcedChargeFlag{segment_id}", 0) - try: - enabled = bool(int(enabled_raw)) - except (ValueError, TypeError): - enabled = False + enabled = bool(int(self.data.get(f"forcedChargeFlag{segment_id}", 0))) return { "segment_id": segment_id, "start_time": start_time, "end_time": end_time, "batt_mode": batt_mode, - "mode_name": mode_name, "enabled": enabled, } diff --git a/homeassistant/components/growatt_server/services.py b/homeassistant/components/growatt_server/services.py new file mode 100644 index 00000000000000..791c146bf5c2c2 --- /dev/null +++ b/homeassistant/components/growatt_server/services.py @@ -0,0 +1,181 @@ +"""Service handlers for Growatt Server integration.""" + +from __future__ import annotations + +from datetime import datetime +from typing import TYPE_CHECKING, Any + +from homeassistant.core import HomeAssistant, ServiceCall, SupportsResponse +from homeassistant.exceptions import ServiceValidationError +from homeassistant.helpers import device_registry as dr + +from .const import ( + BATT_MODE_BATTERY_FIRST, + BATT_MODE_GRID_FIRST, + BATT_MODE_LOAD_FIRST, + DOMAIN, +) + +if TYPE_CHECKING: + from .coordinator import GrowattCoordinator + + +async def async_register_services(hass: HomeAssistant) -> None: + """Register services for Growatt Server integration.""" + + def get_min_coordinators() -> dict[str, GrowattCoordinator]: + """Get all MIN coordinators with V1 API from loaded config entries.""" + min_coordinators: dict[str, GrowattCoordinator] = {} + + for entry in hass.config_entries.async_entries(DOMAIN): + if not entry.runtime_data: + continue + + # Add MIN coordinators from this entry + for coord in entry.runtime_data.devices.values(): + if coord.device_type == "min" and coord.api_version == "v1": + min_coordinators[coord.device_id] = coord + + return min_coordinators + + # Services will be registered even if no MIN devices currently exist, + # but will check for available coordinators when called + if hass.services.has_service(DOMAIN, "update_time_segment"): + # Services already registered + return + + def get_coordinator(device_id: str | None = None) -> GrowattCoordinator: + """Get coordinator by device_id with consistent behavior. + + Args: + device_id: Device registry ID (not serial number) + """ + # Get current coordinators (they may have changed since service registration) + min_coordinators = get_min_coordinators() + + if not min_coordinators: + raise ServiceValidationError( + "No MIN devices with token authentication are configured. " + "Services require MIN devices with V1 API access." + ) + + if device_id is None: + if len(min_coordinators) == 1: + # Only one device - return it + return next(iter(min_coordinators.values())) + # Multiple devices - require explicit selection + raise ServiceValidationError( + "Multiple MIN devices available. Please specify a device." + ) + + # Device registry ID provided - map to serial number + device_registry: dr.DeviceRegistry = dr.async_get(hass) + device_entry: dr.DeviceEntry | None = device_registry.async_get(device_id) + + if not device_entry: + raise ServiceValidationError(f"Device '{device_id}' not found") + + # Extract serial number from device identifiers + serial_number: str | None = None + for identifier in device_entry.identifiers: + if identifier[0] == DOMAIN: + serial_number = identifier[1] + break + + if not serial_number: + raise ServiceValidationError( + f"Device '{device_id}' is not a Growatt device" + ) + + # Find coordinator by serial number + if serial_number not in min_coordinators: + raise ServiceValidationError( + f"MIN device '{serial_number}' not found or not configured for services" + ) + + return min_coordinators[serial_number] + + async def handle_update_time_segment(call: ServiceCall) -> None: + """Handle update_time_segment service call.""" + segment_id: int = int(call.data["segment_id"]) + batt_mode_str: str = call.data["batt_mode"] + start_time_str: str = call.data["start_time"] + end_time_str: str = call.data["end_time"] + enabled: bool = call.data["enabled"] + device_id: str | None = call.data.get("device_id") + + # Validate segment_id range + if not 1 <= segment_id <= 9: + raise ServiceValidationError( + f"segment_id must be between 1 and 9, got {segment_id}" + ) + + # Validate and convert batt_mode string to integer + valid_modes = { + "load_first": BATT_MODE_LOAD_FIRST, + "battery_first": BATT_MODE_BATTERY_FIRST, + "grid_first": BATT_MODE_GRID_FIRST, + } + if batt_mode_str not in valid_modes: + raise ServiceValidationError( + f"batt_mode must be one of {list(valid_modes.keys())}, got '{batt_mode_str}'" + ) + batt_mode: int = valid_modes[batt_mode_str] + + # Convert time strings to datetime.time objects + # UI time selector sends HH:MM:SS, but we only need HH:MM (strip seconds) + try: + # Take only HH:MM part (ignore seconds if present) + start_parts = start_time_str.split(":") + start_time_hhmm = f"{start_parts[0]}:{start_parts[1]}" + start_time = datetime.strptime(start_time_hhmm, "%H:%M").time() + except (ValueError, IndexError) as err: + raise ServiceValidationError( + "start_time must be in HH:MM or HH:MM:SS format" + ) from err + + try: + # Take only HH:MM part (ignore seconds if present) + end_parts = end_time_str.split(":") + end_time_hhmm = f"{end_parts[0]}:{end_parts[1]}" + end_time = datetime.strptime(end_time_hhmm, "%H:%M").time() + except (ValueError, IndexError) as err: + raise ServiceValidationError( + "end_time must be in HH:MM or HH:MM:SS format" + ) from err + + # Get the appropriate MIN coordinator + coordinator: GrowattCoordinator = get_coordinator(device_id) + + await coordinator.update_time_segment( + segment_id, + batt_mode, + start_time, + end_time, + enabled, + ) + + async def handle_read_time_segments(call: ServiceCall) -> dict[str, Any]: + """Handle read_time_segments service call.""" + # Get the appropriate MIN coordinator + coordinator: GrowattCoordinator = get_coordinator(call.data.get("device_id")) + + time_segments: list[dict[str, Any]] = await coordinator.read_time_segments() + + return {"time_segments": time_segments} + + # Register services without schema - services.yaml will provide UI definition + # Schema validation happens in the handler functions + hass.services.async_register( + DOMAIN, + "update_time_segment", + handle_update_time_segment, + supports_response=SupportsResponse.NONE, + ) + + hass.services.async_register( + DOMAIN, + "read_time_segments", + handle_read_time_segments, + supports_response=SupportsResponse.ONLY, + ) diff --git a/homeassistant/components/growatt_server/services.yaml b/homeassistant/components/growatt_server/services.yaml index b18c792bbabd4f..60c88eba621deb 100644 --- a/homeassistant/components/growatt_server/services.yaml +++ b/homeassistant/components/growatt_server/services.yaml @@ -1,5 +1,4 @@ # Service definitions for Growatt Server integration -# Schemas are defined dynamically in code (__init__.py) using selectors update_time_segment: fields: @@ -13,13 +12,14 @@ update_time_segment: mode: box batt_mode: required: true - example: "load-first" + example: "load_first" selector: select: options: - - "load-first" - - "battery-first" - - "grid-first" + - "load_first" + - "battery_first" + - "grid_first" + translation_key: batt_mode start_time: required: true example: "08:00" @@ -37,14 +37,14 @@ update_time_segment: boolean: device_id: required: false - example: "MIN12345" selector: - text: + device: + integration: growatt_server read_time_segments: fields: device_id: required: false - example: "MIN12345" selector: - text: + device: + integration: growatt_server diff --git a/homeassistant/components/growatt_server/strings.json b/homeassistant/components/growatt_server/strings.json index d5f1a069820304..a8f741f1f7476e 100644 --- a/homeassistant/components/growatt_server/strings.json +++ b/homeassistant/components/growatt_server/strings.json @@ -509,6 +509,15 @@ } } }, + "selector": { + "batt_mode": { + "options": { + "battery_first": "Battery first", + "grid_first": "Grid first", + "load_first": "Load first" + } + } + }, "services": { "read_time_segments": { "description": "Read all time-of-use (TOU) segments from a supported inverter.", diff --git a/tests/components/growatt_server/test_services.py b/tests/components/growatt_server/test_services.py index 495ac082c83876..04bf0559b47ec2 100644 --- a/tests/components/growatt_server/test_services.py +++ b/tests/components/growatt_server/test_services.py @@ -4,11 +4,11 @@ import growattServer import pytest -import voluptuous as vol from homeassistant.components.growatt_server.const import DOMAIN from homeassistant.core import HomeAssistant -from homeassistant.exceptions import HomeAssistantError +from homeassistant.exceptions import HomeAssistantError, ServiceValidationError +from homeassistant.helpers import device_registry as dr from tests.common import MockConfigEntry @@ -71,7 +71,7 @@ async def test_update_time_segment_charge_mode( "segment_id": 1, "start_time": "09:00", "end_time": "11:00", - "batt_mode": "load-first", + "batt_mode": "load_first", "enabled": True, }, blocking=True, @@ -106,7 +106,7 @@ async def test_update_time_segment_discharge_mode( "segment_id": 2, "start_time": "14:00", "end_time": "16:00", - "batt_mode": "battery-first", + "batt_mode": "battery_first", "enabled": True, }, blocking=True, @@ -140,7 +140,7 @@ async def test_update_time_segment_standby_mode( "segment_id": 3, "start_time": "20:00", "end_time": "22:00", - "batt_mode": "grid-first", + "batt_mode": "grid_first", "enabled": True, }, blocking=True, @@ -174,7 +174,7 @@ async def test_update_time_segment_disabled( "segment_id": 1, "start_time": "06:00", "end_time": "08:00", - "batt_mode": "load-first", + "batt_mode": "load_first", "enabled": False, }, blocking=True, @@ -183,6 +183,41 @@ async def test_update_time_segment_disabled( mock_growatt_v1_api.min_write_time_segment.assert_called_once() +async def test_update_time_segment_with_seconds( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_v1_api, +) -> None: + """Test updating time segment with HH:MM:SS format from UI.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Test with HH:MM:SS format (what the UI time selector sends) + await hass.services.async_call( + DOMAIN, + "update_time_segment", + { + "segment_id": 1, + "start_time": "09:00:00", + "end_time": "11:00:00", + "batt_mode": "load_first", + "enabled": True, + }, + blocking=True, + ) + + mock_growatt_v1_api.min_write_time_segment.assert_called_once() + + async def test_update_time_segment_api_error( hass: HomeAssistant, mock_config_entry: MockConfigEntry, @@ -210,7 +245,7 @@ async def test_update_time_segment_api_error( ) ) - with pytest.raises(HomeAssistantError, match="Error updating time segment"): + with pytest.raises(HomeAssistantError, match="API error updating time segment"): await hass.services.async_call( DOMAIN, "update_time_segment", @@ -218,7 +253,7 @@ async def test_update_time_segment_api_error( "segment_id": 1, "start_time": "09:00", "end_time": "11:00", - "batt_mode": "load-first", + "batt_mode": "load_first", "enabled": True, }, blocking=True, @@ -230,7 +265,7 @@ async def test_no_min_devices_skips_service_registration( mock_config_entry_classic: MockConfigEntry, mock_growatt_classic_api, ) -> None: - """Test that no services are registered when no MIN devices exist.""" + """Test that services fail gracefully when no MIN devices exist.""" mock_config_entry_classic.add_to_hass(hass) with patch( @@ -244,9 +279,26 @@ async def test_no_min_devices_skips_service_registration( assert await hass.config_entries.async_setup(mock_config_entry_classic.entry_id) await hass.async_block_till_done() - # Verify services are not registered - assert not hass.services.has_service(DOMAIN, "update_time_segment") - assert not hass.services.has_service(DOMAIN, "read_time_segments") + # Verify services are registered (they're always registered in async_setup) + assert hass.services.has_service(DOMAIN, "update_time_segment") + assert hass.services.has_service(DOMAIN, "read_time_segments") + + # But calling them should fail with appropriate error + with pytest.raises( + ServiceValidationError, match="No MIN devices with token authentication" + ): + await hass.services.async_call( + DOMAIN, + "update_time_segment", + { + "segment_id": 1, + "start_time": "09:00", + "end_time": "11:00", + "batt_mode": "load_first", + "enabled": True, + }, + blocking=True, + ) async def test_multiple_devices_require_device_id_in_schema( @@ -254,7 +306,7 @@ async def test_multiple_devices_require_device_id_in_schema( mock_config_entry: MockConfigEntry, mock_growatt_v1_api, ) -> None: - """Test that multiple devices require device_id parameter in schema.""" + """Test that multiple devices require device_id parameter at runtime.""" mock_config_entry.add_to_hass(hass) with patch( @@ -274,8 +326,8 @@ async def test_multiple_devices_require_device_id_in_schema( assert hass.services.has_service(DOMAIN, "update_time_segment") assert hass.services.has_service(DOMAIN, "read_time_segments") - # Test that schema validation requires device_id for update service - with pytest.raises(vol.MultipleInvalid, match="required key not provided"): + # Test that runtime validation requires device_id for update service + with pytest.raises(ServiceValidationError, match="Multiple MIN devices available"): await hass.services.async_call( DOMAIN, "update_time_segment", @@ -284,14 +336,14 @@ async def test_multiple_devices_require_device_id_in_schema( "segment_id": 1, "start_time": "09:00", "end_time": "11:00", - "batt_mode": "load-first", + "batt_mode": "load_first", "enabled": True, }, blocking=True, ) - # Test that schema validation requires device_id for read service - with pytest.raises(vol.MultipleInvalid, match="required key not provided"): + # Test that runtime validation requires device_id for read service + with pytest.raises(ServiceValidationError, match="Multiple MIN devices available"): await hass.services.async_call( DOMAIN, "read_time_segments", @@ -327,7 +379,7 @@ async def test_single_device_does_not_require_device_id( "segment_id": 1, "start_time": "09:00", "end_time": "11:00", - "batt_mode": "load-first", + "batt_mode": "load_first", "enabled": True, }, blocking=True, @@ -352,6 +404,7 @@ async def test_multiple_devices_with_valid_device_id_works( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test that multiple devices work when device_id is specified.""" mock_config_entry.add_to_hass(hass) @@ -369,16 +422,20 @@ async def test_multiple_devices_with_valid_device_id_works( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() - # Test update service with specific device_id + # Get the device registry ID for the first MIN device + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + + # Test update service with specific device_id (device registry ID) await hass.services.async_call( DOMAIN, "update_time_segment", { - "device_id": "MIN123456", + "device_id": device_entry.id, "segment_id": 1, "start_time": "09:00", "end_time": "11:00", - "batt_mode": "load-first", + "batt_mode": "load_first", "enabled": True, }, blocking=True, @@ -386,11 +443,11 @@ async def test_multiple_devices_with_valid_device_id_works( mock_growatt_v1_api.min_write_time_segment.assert_called_once() - # Test read service with specific device_id + # Test read service with specific device_id (device registry ID) response = await hass.services.async_call( DOMAIN, "read_time_segments", - {"device_id": "MIN123456"}, + {"device_id": device_entry.id}, blocking=True, return_response=True, ) @@ -417,8 +474,10 @@ async def test_update_time_segment_invalid_time_format( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() - # Test with invalid time format - schema validates before handler sees it - with pytest.raises(vol.MultipleInvalid, match="Invalid time specified"): + # Test with invalid time format + with pytest.raises( + ServiceValidationError, match="start_time must be in HH:MM or HH:MM:SS format" + ): await hass.services.async_call( DOMAIN, "update_time_segment", @@ -426,7 +485,94 @@ async def test_update_time_segment_invalid_time_format( "segment_id": 1, "start_time": "invalid", "end_time": "11:00", - "batt_mode": "load-first", + "batt_mode": "load_first", + "enabled": True, + }, + blocking=True, + ) + + +async def test_update_time_segment_invalid_segment_id( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_v1_api, +) -> None: + """Test validation of segment_id range.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Test segment_id too low + with pytest.raises( + ServiceValidationError, match="segment_id must be between 1 and 9" + ): + await hass.services.async_call( + DOMAIN, + "update_time_segment", + { + "segment_id": 0, + "start_time": "09:00", + "end_time": "11:00", + "batt_mode": "load_first", + "enabled": True, + }, + blocking=True, + ) + + # Test segment_id too high + with pytest.raises( + ServiceValidationError, match="segment_id must be between 1 and 9" + ): + await hass.services.async_call( + DOMAIN, + "update_time_segment", + { + "segment_id": 10, + "start_time": "09:00", + "end_time": "11:00", + "batt_mode": "load_first", + "enabled": True, + }, + blocking=True, + ) + + +async def test_update_time_segment_invalid_batt_mode( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_growatt_v1_api, +) -> None: + """Test validation of batt_mode value.""" + mock_config_entry.add_to_hass(hass) + + with patch( + "homeassistant.components.growatt_server.get_device_list" + ) as mock_get_devices: + mock_get_devices.return_value = ( + [{"deviceSn": "MIN123456", "deviceType": "min"}], + "12345", + ) + assert await hass.config_entries.async_setup(mock_config_entry.entry_id) + await hass.async_block_till_done() + + # Test invalid batt_mode + with pytest.raises(ServiceValidationError, match="batt_mode must be one of"): + await hass.services.async_call( + DOMAIN, + "update_time_segment", + { + "segment_id": 1, + "start_time": "09:00", + "end_time": "11:00", + "batt_mode": "invalid_mode", "enabled": True, }, blocking=True, @@ -457,7 +603,7 @@ async def test_read_time_segments_api_error( "homeassistant.components.growatt_server.coordinator.GrowattCoordinator.read_time_segments", side_effect=HomeAssistantError("API connection failed"), ), - pytest.raises(HomeAssistantError, match="Error reading time segments"), + pytest.raises(HomeAssistantError, match="API connection failed"), ): await hass.services.async_call( DOMAIN, From 1f723edf665620e4b25710341b9d5f976c224395 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Thu, 30 Oct 2025 20:35:16 +0000 Subject: [PATCH 08/17] Restore number platform content in strings.json and conftest.py --- .../components/growatt_server/strings.json | 14 +++++++++ tests/components/growatt_server/conftest.py | 31 ++++++++++++++----- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/homeassistant/components/growatt_server/strings.json b/homeassistant/components/growatt_server/strings.json index a8f741f1f7476e..8e608d24280de4 100644 --- a/homeassistant/components/growatt_server/strings.json +++ b/homeassistant/components/growatt_server/strings.json @@ -42,6 +42,20 @@ } }, "entity": { + "number": { + "battery_charge_power_limit": { + "name": "Battery charge power limit" + }, + "battery_charge_soc_limit": { + "name": "Battery charge SOC limit" + }, + "battery_discharge_power_limit": { + "name": "Battery discharge power limit" + }, + "battery_discharge_soc_limit": { + "name": "Battery discharge SOC limit" + } + }, "sensor": { "inverter_amperage_input_1": { "name": "Input 1 amperage" diff --git a/tests/components/growatt_server/conftest.py b/tests/components/growatt_server/conftest.py index c14662dc5950da..0cc9e23c7be8e1 100644 --- a/tests/components/growatt_server/conftest.py +++ b/tests/components/growatt_server/conftest.py @@ -30,12 +30,12 @@ def mock_growatt_v1_api(): - plant_energy_overview: Called by total coordinator during first refresh Methods mocked for MIN device coordinator refresh: - - min_detail: Provides device state (e.g., acChargeEnable for switches) + - min_detail: Provides device state (e.g., acChargeEnable, chargePowerCommand) - min_settings: Provides settings (e.g. TOU periods) - - min_energy: Provides energy data (empty for switch tests, sensors need real data) + - min_energy: Provides energy data (empty for switch/number tests, sensors need real data) - Methods mocked for switch operations: - - min_write_parameter: Called by switch entities to change settings + Methods mocked for switch and number operations: + - min_write_parameter: Called by switch/number entities to change settings Methods mocked for service operations: - min_write_time_segment: Called by time segment management services @@ -57,11 +57,15 @@ def mock_growatt_v1_api(): mock_v1_api.min_detail.return_value = { "deviceSn": "MIN123456", "acChargeEnable": 1, # AC charge enabled - read by switch entity + "chargePowerCommand": 50, # 50% charge power - read by number entity + "wchargeSOCLowLimit": 10, # 10% charge stop SOC - read by number entity + "disChargePowerCommand": 80, # 80% discharge power - read by number entity + "wdisChargeSOCLowLimit": 20, # 20% discharge stop SOC - read by number entity } # Called by MIN device coordinator during refresh mock_v1_api.min_settings.return_value = { - # Segment 1 - enabled + # Forced charge time segments (not used by switch/number, but coordinator fetches it) "forcedTimeStart1": "06:00", "forcedTimeStop1": "08:00", "forcedChargeBatMode1": 1, @@ -74,8 +78,19 @@ def mock_growatt_v1_api(): } # Called by MIN device coordinator during refresh - # Empty dict is sufficient for switch tests (sensor tests would need real energy data) - mock_v1_api.min_energy.return_value = {} + # Provide realistic energy data for sensor tests + mock_v1_api.min_energy.return_value = { + "eChargeToday": 5.2, + "eChargeTotal": 125.8, + "eDischargeToday": 8.1, + "eDischargeTotal": 245.6, + "eSelfToday": 12.5, + "eSelfTotal": 320.4, + "eBatChargeToday": 6.3, + "eBatChargeTotal": 150.2, + "eBatDischargeToday": 7.8, + "eBatDischargeTotal": 180.5, + } # Called by total coordinator during refresh mock_v1_api.plant_energy_overview.return_value = { @@ -84,7 +99,7 @@ def mock_growatt_v1_api(): "current_power": 2500, } - # Called by switch entities during turn_on/turn_off + # Called by switch/number entities during turn_on/turn_off/set_value mock_v1_api.min_write_parameter.return_value = None # Called by time segment management services From 830158d656374fd3e4a60c3f3088601887e5faf2 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Thu, 30 Oct 2025 20:56:31 +0000 Subject: [PATCH 09/17] Fix stale data issue: refresh coordinator after updating time segment After writing a time segment value, the coordinator data was not refreshed, causing read_time_segments to return stale/old values. Now triggers an async_refresh() after successful write to ensure subsequent reads return the updated values. --- homeassistant/components/growatt_server/coordinator.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/homeassistant/components/growatt_server/coordinator.py b/homeassistant/components/growatt_server/coordinator.py index 22f8bfd7709848..8bc2db82ed996c 100644 --- a/homeassistant/components/growatt_server/coordinator.py +++ b/homeassistant/components/growatt_server/coordinator.py @@ -301,6 +301,9 @@ async def update_time_segment( except growattServer.GrowattV1ApiError as err: raise HomeAssistantError(f"API error updating time segment: {err}") from err + # Trigger a refresh to update the data with the new values + await self.async_refresh() + async def read_time_segments(self) -> list[dict]: """Read time segments from an inverter. From cc76d6c5135a083ce380674b4d1f3b1e078de24b Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Thu, 30 Oct 2025 20:59:21 +0000 Subject: [PATCH 10/17] Revert "Fix stale data issue: refresh coordinator after updating time segment" This reverts commit 830158d656374fd3e4a60c3f3088601887e5faf2. --- homeassistant/components/growatt_server/coordinator.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/homeassistant/components/growatt_server/coordinator.py b/homeassistant/components/growatt_server/coordinator.py index 8bc2db82ed996c..22f8bfd7709848 100644 --- a/homeassistant/components/growatt_server/coordinator.py +++ b/homeassistant/components/growatt_server/coordinator.py @@ -301,9 +301,6 @@ async def update_time_segment( except growattServer.GrowattV1ApiError as err: raise HomeAssistantError(f"API error updating time segment: {err}") from err - # Trigger a refresh to update the data with the new values - await self.async_refresh() - async def read_time_segments(self) -> list[dict]: """Read time segments from an inverter. From 7b4637bb0d6bf01391b73987637beece58f5dbcc Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Thu, 30 Oct 2025 21:03:16 +0000 Subject: [PATCH 11/17] Update coordinator cache after writing time segment to avoid stale data After writing a time segment value, update the coordinator's cached data directly without making an additional API call. This ensures that subsequent read_time_segments calls return the updated values immediately while avoiding rate limit issues. Uses async_set_updated_data() to update the cache and notify entities of the change without triggering a coordinator refresh. --- .../components/growatt_server/coordinator.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/homeassistant/components/growatt_server/coordinator.py b/homeassistant/components/growatt_server/coordinator.py index 22f8bfd7709848..fbd4a7c4bfbe52 100644 --- a/homeassistant/components/growatt_server/coordinator.py +++ b/homeassistant/components/growatt_server/coordinator.py @@ -301,6 +301,17 @@ async def update_time_segment( except growattServer.GrowattV1ApiError as err: raise HomeAssistantError(f"API error updating time segment: {err}") from err + # Update coordinator's cached data without making an API call (avoids rate limit) + if self.data: + # Update the time segment data in the cache + self.data[f"forcedTimeStart{segment_id}"] = start_time.strftime("%H:%M") + self.data[f"forcedTimeStop{segment_id}"] = end_time.strftime("%H:%M") + self.data[f"forcedChargeBatMode{segment_id}"] = batt_mode + self.data[f"forcedChargeFlag{segment_id}"] = 1 if enabled else 0 + + # Notify entities of the updated data (no API call) + self.async_set_updated_data(self.data) + async def read_time_segments(self) -> list[dict]: """Read time segments from an inverter. From 6eaccc6ca55781eab5e2097938aa7fa9b96aaeea Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Thu, 30 Oct 2025 21:14:06 +0000 Subject: [PATCH 12/17] Fix unique_id in conftest.py back to plant_123 Restore unique_id to 'plant_123' (was accidentally changed to '12345' during merge conflict resolution). This matches the upstream/dev version and the plant_id in the config data. --- tests/components/growatt_server/conftest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/components/growatt_server/conftest.py b/tests/components/growatt_server/conftest.py index 0cc9e23c7be8e1..dec4da301dfb7a 100644 --- a/tests/components/growatt_server/conftest.py +++ b/tests/components/growatt_server/conftest.py @@ -183,7 +183,7 @@ def mock_config_entry() -> MockConfigEntry: CONF_PLANT_ID: "plant_123", "name": "Test Plant", }, - unique_id="12345", + unique_id="plant_123", ) From 456280b7b1b56984213a0c39e74ba7d037b156ef Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Sun, 2 Nov 2025 10:04:35 +0000 Subject: [PATCH 13/17] Adress review comments --- .../components/growatt_server/services.py | 34 +--- .../components/growatt_server/services.yaml | 4 +- .../components/growatt_server/strings.json | 12 +- .../growatt_server/test_services.py | 189 ++++++++---------- 4 files changed, 100 insertions(+), 139 deletions(-) diff --git a/homeassistant/components/growatt_server/services.py b/homeassistant/components/growatt_server/services.py index 791c146bf5c2c2..ecdae8ed3f672b 100644 --- a/homeassistant/components/growatt_server/services.py +++ b/homeassistant/components/growatt_server/services.py @@ -5,6 +5,7 @@ from datetime import datetime from typing import TYPE_CHECKING, Any +from homeassistant.config_entries import ConfigEntryState from homeassistant.core import HomeAssistant, ServiceCall, SupportsResponse from homeassistant.exceptions import ServiceValidationError from homeassistant.helpers import device_registry as dr @@ -28,7 +29,7 @@ def get_min_coordinators() -> dict[str, GrowattCoordinator]: min_coordinators: dict[str, GrowattCoordinator] = {} for entry in hass.config_entries.async_entries(DOMAIN): - if not entry.runtime_data: + if entry.state != ConfigEntryState.LOADED: continue # Add MIN coordinators from this entry @@ -38,14 +39,8 @@ def get_min_coordinators() -> dict[str, GrowattCoordinator]: return min_coordinators - # Services will be registered even if no MIN devices currently exist, - # but will check for available coordinators when called - if hass.services.has_service(DOMAIN, "update_time_segment"): - # Services already registered - return - - def get_coordinator(device_id: str | None = None) -> GrowattCoordinator: - """Get coordinator by device_id with consistent behavior. + def get_coordinator(device_id: str) -> GrowattCoordinator: + """Get coordinator by device_id. Args: device_id: Device registry ID (not serial number) @@ -59,24 +54,15 @@ def get_coordinator(device_id: str | None = None) -> GrowattCoordinator: "Services require MIN devices with V1 API access." ) - if device_id is None: - if len(min_coordinators) == 1: - # Only one device - return it - return next(iter(min_coordinators.values())) - # Multiple devices - require explicit selection - raise ServiceValidationError( - "Multiple MIN devices available. Please specify a device." - ) - # Device registry ID provided - map to serial number - device_registry: dr.DeviceRegistry = dr.async_get(hass) - device_entry: dr.DeviceEntry | None = device_registry.async_get(device_id) + device_registry = dr.async_get(hass) + device_entry = device_registry.async_get(device_id) if not device_entry: raise ServiceValidationError(f"Device '{device_id}' not found") # Extract serial number from device identifiers - serial_number: str | None = None + serial_number = None for identifier in device_entry.identifiers: if identifier[0] == DOMAIN: serial_number = identifier[1] @@ -102,7 +88,7 @@ async def handle_update_time_segment(call: ServiceCall) -> None: start_time_str: str = call.data["start_time"] end_time_str: str = call.data["end_time"] enabled: bool = call.data["enabled"] - device_id: str | None = call.data.get("device_id") + device_id: str = call.data["device_id"] # Validate segment_id range if not 1 <= segment_id <= 9: @@ -157,8 +143,10 @@ async def handle_update_time_segment(call: ServiceCall) -> None: async def handle_read_time_segments(call: ServiceCall) -> dict[str, Any]: """Handle read_time_segments service call.""" + device_id: str = call.data["device_id"] + # Get the appropriate MIN coordinator - coordinator: GrowattCoordinator = get_coordinator(call.data.get("device_id")) + coordinator: GrowattCoordinator = get_coordinator(device_id) time_segments: list[dict[str, Any]] = await coordinator.read_time_segments() diff --git a/homeassistant/components/growatt_server/services.yaml b/homeassistant/components/growatt_server/services.yaml index 60c88eba621deb..318ab71aad07f9 100644 --- a/homeassistant/components/growatt_server/services.yaml +++ b/homeassistant/components/growatt_server/services.yaml @@ -36,7 +36,7 @@ update_time_segment: selector: boolean: device_id: - required: false + required: true selector: device: integration: growatt_server @@ -44,7 +44,7 @@ update_time_segment: read_time_segments: fields: device_id: - required: false + required: true selector: device: integration: growatt_server diff --git a/homeassistant/components/growatt_server/strings.json b/homeassistant/components/growatt_server/strings.json index 8e608d24280de4..d5d2278bd5d839 100644 --- a/homeassistant/components/growatt_server/strings.json +++ b/homeassistant/components/growatt_server/strings.json @@ -534,25 +534,25 @@ }, "services": { "read_time_segments": { - "description": "Read all time-of-use (TOU) segments from a supported inverter.", + "description": "Read all time segments from a supported inverter.", "fields": { "device_id": { - "description": "Device ID (only required when multiple devices exist).", - "name": "Device ID" + "description": "The Growatt device to read time segments from.", + "name": "Device" } }, "name": "Read time segments" }, "update_time_segment": { - "description": "Update a time-of-use (TOU) segment for supported inverters.", + "description": "Update a time segment for supported inverters.", "fields": { "batt_mode": { "description": "Battery operation mode for this time segment.", "name": "Battery mode" }, "device_id": { - "description": "Device ID (only required when multiple devices exist).", - "name": "Device ID" + "description": "[%key:component::growatt_server::services::read_time_segments::fields::device_id::description%]", + "name": "[%key:component::growatt_server::services::read_time_segments::fields::device_id::name%]" }, "enabled": { "description": "Whether this time segment is active.", diff --git a/tests/components/growatt_server/test_services.py b/tests/components/growatt_server/test_services.py index 04bf0559b47ec2..8c45ea11358956 100644 --- a/tests/components/growatt_server/test_services.py +++ b/tests/components/growatt_server/test_services.py @@ -13,10 +13,11 @@ from tests.common import MockConfigEntry +@pytest.mark.usefixtures("mock_growatt_v1_api") async def test_read_time_segments_single_device( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test reading time segments for single device.""" mock_config_entry.add_to_hass(hass) @@ -31,17 +32,22 @@ async def test_read_time_segments_single_device( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + # Test service call response = await hass.services.async_call( DOMAIN, "read_time_segments", - {}, + {"device_id": device_entry.id}, blocking=True, return_response=True, ) assert response is not None assert "time_segments" in response + assert isinstance(response["time_segments"], list) assert len(response["time_segments"]) == 9 # Returns all 9 segments (1-9) @@ -49,6 +55,7 @@ async def test_update_time_segment_charge_mode( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test updating time segment with charge mode.""" mock_config_entry.add_to_hass(hass) @@ -63,11 +70,16 @@ async def test_update_time_segment_charge_mode( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + # Test successful update await hass.services.async_call( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 1, "start_time": "09:00", "end_time": "11:00", @@ -85,6 +97,7 @@ async def test_update_time_segment_discharge_mode( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test updating time segment with discharge mode.""" mock_config_entry.add_to_hass(hass) @@ -99,10 +112,15 @@ async def test_update_time_segment_discharge_mode( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + await hass.services.async_call( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 2, "start_time": "14:00", "end_time": "16:00", @@ -119,6 +137,7 @@ async def test_update_time_segment_standby_mode( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test updating time segment with standby mode.""" mock_config_entry.add_to_hass(hass) @@ -133,10 +152,15 @@ async def test_update_time_segment_standby_mode( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + await hass.services.async_call( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 3, "start_time": "20:00", "end_time": "22:00", @@ -153,6 +177,7 @@ async def test_update_time_segment_disabled( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test disabling a time segment.""" mock_config_entry.add_to_hass(hass) @@ -167,10 +192,15 @@ async def test_update_time_segment_disabled( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + await hass.services.async_call( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 1, "start_time": "06:00", "end_time": "08:00", @@ -187,6 +217,7 @@ async def test_update_time_segment_with_seconds( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test updating time segment with HH:MM:SS format from UI.""" mock_config_entry.add_to_hass(hass) @@ -201,11 +232,16 @@ async def test_update_time_segment_with_seconds( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + # Test with HH:MM:SS format (what the UI time selector sends) await hass.services.async_call( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 1, "start_time": "09:00:00", "end_time": "11:00:00", @@ -222,6 +258,7 @@ async def test_update_time_segment_api_error( hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test handling API error when updating time segment.""" mock_config_entry.add_to_hass(hass) @@ -236,6 +273,10 @@ async def test_update_time_segment_api_error( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + # Mock API error - the library raises an exception instead of returning error dict mock_growatt_v1_api.min_write_time_segment.side_effect = ( growattServer.GrowattV1ApiError( @@ -250,6 +291,7 @@ async def test_update_time_segment_api_error( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 1, "start_time": "09:00", "end_time": "11:00", @@ -260,10 +302,11 @@ async def test_update_time_segment_api_error( ) +@pytest.mark.usefixtures("mock_growatt_classic_api") async def test_no_min_devices_skips_service_registration( hass: HomeAssistant, mock_config_entry_classic: MockConfigEntry, - mock_growatt_classic_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test that services fail gracefully when no MIN devices exist.""" mock_config_entry_classic.add_to_hass(hass) @@ -283,7 +326,11 @@ async def test_no_min_devices_skips_service_registration( assert hass.services.has_service(DOMAIN, "update_time_segment") assert hass.services.has_service(DOMAIN, "read_time_segments") - # But calling them should fail with appropriate error + # Get the TLX device (non-MIN) + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "TLX123456")}) + assert device_entry is not None + + # But calling them with a non-MIN device should fail with appropriate error with pytest.raises( ServiceValidationError, match="No MIN devices with token authentication" ): @@ -291,6 +338,7 @@ async def test_no_min_devices_skips_service_registration( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 1, "start_time": "09:00", "end_time": "11:00", @@ -301,105 +349,6 @@ async def test_no_min_devices_skips_service_registration( ) -async def test_multiple_devices_require_device_id_in_schema( - hass: HomeAssistant, - mock_config_entry: MockConfigEntry, - mock_growatt_v1_api, -) -> None: - """Test that multiple devices require device_id parameter at runtime.""" - mock_config_entry.add_to_hass(hass) - - with patch( - "homeassistant.components.growatt_server.get_device_list" - ) as mock_get_devices: - mock_get_devices.return_value = ( - [ - {"deviceSn": "MIN123456", "deviceType": "min"}, - {"deviceSn": "MIN789012", "deviceType": "min"}, - ], - "12345", - ) - assert await hass.config_entries.async_setup(mock_config_entry.entry_id) - await hass.async_block_till_done() - - # Verify services are registered - assert hass.services.has_service(DOMAIN, "update_time_segment") - assert hass.services.has_service(DOMAIN, "read_time_segments") - - # Test that runtime validation requires device_id for update service - with pytest.raises(ServiceValidationError, match="Multiple MIN devices available"): - await hass.services.async_call( - DOMAIN, - "update_time_segment", - { - # Missing required device_id - "segment_id": 1, - "start_time": "09:00", - "end_time": "11:00", - "batt_mode": "load_first", - "enabled": True, - }, - blocking=True, - ) - - # Test that runtime validation requires device_id for read service - with pytest.raises(ServiceValidationError, match="Multiple MIN devices available"): - await hass.services.async_call( - DOMAIN, - "read_time_segments", - {}, # Missing required device_id - blocking=True, - return_response=True, - ) - - -async def test_single_device_does_not_require_device_id( - hass: HomeAssistant, - mock_config_entry: MockConfigEntry, - mock_growatt_v1_api, -) -> None: - """Test that single device works without device_id parameter.""" - mock_config_entry.add_to_hass(hass) - - with patch( - "homeassistant.components.growatt_server.get_device_list" - ) as mock_get_devices: - mock_get_devices.return_value = ( - [{"deviceSn": "MIN123456", "deviceType": "min"}], - "12345", - ) - assert await hass.config_entries.async_setup(mock_config_entry.entry_id) - await hass.async_block_till_done() - - # Test update service works without device_id (single device) - await hass.services.async_call( - DOMAIN, - "update_time_segment", - { - "segment_id": 1, - "start_time": "09:00", - "end_time": "11:00", - "batt_mode": "load_first", - "enabled": True, - }, - blocking=True, - ) - - mock_growatt_v1_api.min_write_time_segment.assert_called() - - # Test read service works without device_id (single device) - response = await hass.services.async_call( - DOMAIN, - "read_time_segments", - {}, - blocking=True, - return_response=True, - ) - - assert response is not None - assert "time_segments" in response - - async def test_multiple_devices_with_valid_device_id_works( hass: HomeAssistant, mock_config_entry: MockConfigEntry, @@ -456,10 +405,11 @@ async def test_multiple_devices_with_valid_device_id_works( assert "time_segments" in response +@pytest.mark.usefixtures("mock_growatt_v1_api") async def test_update_time_segment_invalid_time_format( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test handling invalid time format in update_time_segment.""" mock_config_entry.add_to_hass(hass) @@ -474,6 +424,10 @@ async def test_update_time_segment_invalid_time_format( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + # Test with invalid time format with pytest.raises( ServiceValidationError, match="start_time must be in HH:MM or HH:MM:SS format" @@ -482,6 +436,7 @@ async def test_update_time_segment_invalid_time_format( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 1, "start_time": "invalid", "end_time": "11:00", @@ -492,10 +447,11 @@ async def test_update_time_segment_invalid_time_format( ) +@pytest.mark.usefixtures("mock_growatt_v1_api") async def test_update_time_segment_invalid_segment_id( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test validation of segment_id range.""" mock_config_entry.add_to_hass(hass) @@ -510,6 +466,10 @@ async def test_update_time_segment_invalid_segment_id( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + # Test segment_id too low with pytest.raises( ServiceValidationError, match="segment_id must be between 1 and 9" @@ -518,6 +478,7 @@ async def test_update_time_segment_invalid_segment_id( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 0, "start_time": "09:00", "end_time": "11:00", @@ -535,6 +496,7 @@ async def test_update_time_segment_invalid_segment_id( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 10, "start_time": "09:00", "end_time": "11:00", @@ -545,10 +507,11 @@ async def test_update_time_segment_invalid_segment_id( ) +@pytest.mark.usefixtures("mock_growatt_v1_api") async def test_update_time_segment_invalid_batt_mode( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test validation of batt_mode value.""" mock_config_entry.add_to_hass(hass) @@ -563,12 +526,17 @@ async def test_update_time_segment_invalid_batt_mode( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + # Test invalid batt_mode with pytest.raises(ServiceValidationError, match="batt_mode must be one of"): await hass.services.async_call( DOMAIN, "update_time_segment", { + "device_id": device_entry.id, "segment_id": 1, "start_time": "09:00", "end_time": "11:00", @@ -579,10 +547,11 @@ async def test_update_time_segment_invalid_batt_mode( ) +@pytest.mark.usefixtures("mock_growatt_v1_api") async def test_read_time_segments_api_error( hass: HomeAssistant, mock_config_entry: MockConfigEntry, - mock_growatt_v1_api, + device_registry: dr.DeviceRegistry, ) -> None: """Test handling API error when reading time segments.""" mock_config_entry.add_to_hass(hass) @@ -597,6 +566,10 @@ async def test_read_time_segments_api_error( assert await hass.config_entries.async_setup(mock_config_entry.entry_id) await hass.async_block_till_done() + # Get the device registry ID + device_entry = device_registry.async_get_device(identifiers={(DOMAIN, "MIN123456")}) + assert device_entry is not None + # Mock API error by making coordinator.read_time_segments raise an exception with ( patch( @@ -608,7 +581,7 @@ async def test_read_time_segments_api_error( await hass.services.async_call( DOMAIN, "read_time_segments", - {}, + {"device_id": device_entry.id}, blocking=True, return_response=True, ) From 6cbf358535c3ee2e47025a7648672ab6db486882 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Mon, 3 Nov 2025 17:58:09 +0000 Subject: [PATCH 14/17] Update description for device_id field in time segment service --- homeassistant/components/growatt_server/strings.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/homeassistant/components/growatt_server/strings.json b/homeassistant/components/growatt_server/strings.json index d5d2278bd5d839..5b7eebe556ed91 100644 --- a/homeassistant/components/growatt_server/strings.json +++ b/homeassistant/components/growatt_server/strings.json @@ -537,7 +537,7 @@ "description": "Read all time segments from a supported inverter.", "fields": { "device_id": { - "description": "The Growatt device to read time segments from.", + "description": "The Growatt device to perform the action on.", "name": "Device" } }, From b6b65cd3d3a2340353d61b4481a801e976268fd0 Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Mon, 3 Nov 2025 20:38:21 +0000 Subject: [PATCH 15/17] Fixes bug related to key names for TOU settings --- homeassistant/components/growatt_server/coordinator.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/homeassistant/components/growatt_server/coordinator.py b/homeassistant/components/growatt_server/coordinator.py index fbd4a7c4bfbe52..9bc512205037d3 100644 --- a/homeassistant/components/growatt_server/coordinator.py +++ b/homeassistant/components/growatt_server/coordinator.py @@ -306,8 +306,8 @@ async def update_time_segment( # Update the time segment data in the cache self.data[f"forcedTimeStart{segment_id}"] = start_time.strftime("%H:%M") self.data[f"forcedTimeStop{segment_id}"] = end_time.strftime("%H:%M") - self.data[f"forcedChargeBatMode{segment_id}"] = batt_mode - self.data[f"forcedChargeFlag{segment_id}"] = 1 if enabled else 0 + self.data[f"time{segment_id}Mode"] = batt_mode + self.data[f"forcedStopSwitch{segment_id}"] = 1 if enabled else 0 # Notify entities of the updated data (no API call) self.async_set_updated_data(self.data) @@ -357,7 +357,7 @@ def _parse_time_segment(self, segment_id: int) -> dict: # Get battery mode batt_mode_int = int( - self.data.get(f"forcedChargeBatMode{segment_id}", BATT_MODE_LOAD_FIRST) + self.data.get(f"time{segment_id}Mode", BATT_MODE_LOAD_FIRST) ) # Map numeric mode to string key (matches update_time_segment input format) @@ -369,7 +369,7 @@ def _parse_time_segment(self, segment_id: int) -> dict: batt_mode = mode_map.get(batt_mode_int, "load_first") # Get enabled status - enabled = bool(int(self.data.get(f"forcedChargeFlag{segment_id}", 0))) + enabled = bool(int(self.data.get(f"forcedStopSwitch{segment_id}", 0))) return { "segment_id": segment_id, From a4d0f33547866d84d1444bbc09703441dc7caecd Mon Sep 17 00:00:00 2001 From: Johan Zander Date: Tue, 4 Nov 2025 20:30:05 +0000 Subject: [PATCH 16/17] Add snapshot tests for reading time segments and update mock data --- tests/components/growatt_server/conftest.py | 41 +++++++++-- .../snapshots/test_services.ambr | 70 +++++++++++++++++++ .../growatt_server/test_services.py | 7 +- 3 files changed, 108 insertions(+), 10 deletions(-) create mode 100644 tests/components/growatt_server/snapshots/test_services.ambr diff --git a/tests/components/growatt_server/conftest.py b/tests/components/growatt_server/conftest.py index dec4da301dfb7a..1fa6d4e4086de3 100644 --- a/tests/components/growatt_server/conftest.py +++ b/tests/components/growatt_server/conftest.py @@ -65,16 +65,45 @@ def mock_growatt_v1_api(): # Called by MIN device coordinator during refresh mock_v1_api.min_settings.return_value = { - # Forced charge time segments (not used by switch/number, but coordinator fetches it) + # Time segment 1 - enabled, load_first mode "forcedTimeStart1": "06:00", "forcedTimeStop1": "08:00", - "forcedChargeBatMode1": 1, - "forcedChargeFlag1": 1, - # Segment 2 - disabled + "time1Mode": 1, # load_first + "forcedStopSwitch1": 1, # enabled + # Time segment 2 - disabled "forcedTimeStart2": "22:00", "forcedTimeStop2": "24:00", - "forcedChargeBatMode2": 0, - "forcedChargeFlag2": 0, + "time2Mode": 0, # battery_first + "forcedStopSwitch2": 0, # disabled + # Time segments 3-9 - all disabled with default values + "forcedTimeStart3": "00:00", + "forcedTimeStop3": "00:00", + "time3Mode": 1, + "forcedStopSwitch3": 0, + "forcedTimeStart4": "00:00", + "forcedTimeStop4": "00:00", + "time4Mode": 1, + "forcedStopSwitch4": 0, + "forcedTimeStart5": "00:00", + "forcedTimeStop5": "00:00", + "time5Mode": 1, + "forcedStopSwitch5": 0, + "forcedTimeStart6": "00:00", + "forcedTimeStop6": "00:00", + "time6Mode": 1, + "forcedStopSwitch6": 0, + "forcedTimeStart7": "00:00", + "forcedTimeStop7": "00:00", + "time7Mode": 1, + "forcedStopSwitch7": 0, + "forcedTimeStart8": "00:00", + "forcedTimeStop8": "00:00", + "time8Mode": 1, + "forcedStopSwitch8": 0, + "forcedTimeStart9": "00:00", + "forcedTimeStop9": "00:00", + "time9Mode": 1, + "forcedStopSwitch9": 0, } # Called by MIN device coordinator during refresh diff --git a/tests/components/growatt_server/snapshots/test_services.ambr b/tests/components/growatt_server/snapshots/test_services.ambr new file mode 100644 index 00000000000000..ff43b89d231ead --- /dev/null +++ b/tests/components/growatt_server/snapshots/test_services.ambr @@ -0,0 +1,70 @@ +# serializer version: 1 +# name: test_read_time_segments_single_device + dict({ + 'time_segments': list([ + dict({ + 'batt_mode': 'battery_first', + 'enabled': True, + 'end_time': '08:00', + 'segment_id': 1, + 'start_time': '06:00', + }), + dict({ + 'batt_mode': 'load_first', + 'enabled': False, + 'end_time': '24:00', + 'segment_id': 2, + 'start_time': '22:00', + }), + dict({ + 'batt_mode': 'battery_first', + 'enabled': False, + 'end_time': '00:00', + 'segment_id': 3, + 'start_time': '00:00', + }), + dict({ + 'batt_mode': 'battery_first', + 'enabled': False, + 'end_time': '00:00', + 'segment_id': 4, + 'start_time': '00:00', + }), + dict({ + 'batt_mode': 'battery_first', + 'enabled': False, + 'end_time': '00:00', + 'segment_id': 5, + 'start_time': '00:00', + }), + dict({ + 'batt_mode': 'battery_first', + 'enabled': False, + 'end_time': '00:00', + 'segment_id': 6, + 'start_time': '00:00', + }), + dict({ + 'batt_mode': 'battery_first', + 'enabled': False, + 'end_time': '00:00', + 'segment_id': 7, + 'start_time': '00:00', + }), + dict({ + 'batt_mode': 'battery_first', + 'enabled': False, + 'end_time': '00:00', + 'segment_id': 8, + 'start_time': '00:00', + }), + dict({ + 'batt_mode': 'battery_first', + 'enabled': False, + 'end_time': '00:00', + 'segment_id': 9, + 'start_time': '00:00', + }), + ]), + }) +# --- diff --git a/tests/components/growatt_server/test_services.py b/tests/components/growatt_server/test_services.py index 8c45ea11358956..cd181e055970db 100644 --- a/tests/components/growatt_server/test_services.py +++ b/tests/components/growatt_server/test_services.py @@ -4,6 +4,7 @@ import growattServer import pytest +from syrupy.assertion import SnapshotAssertion from homeassistant.components.growatt_server.const import DOMAIN from homeassistant.core import HomeAssistant @@ -18,6 +19,7 @@ async def test_read_time_segments_single_device( hass: HomeAssistant, mock_config_entry: MockConfigEntry, device_registry: dr.DeviceRegistry, + snapshot: SnapshotAssertion, ) -> None: """Test reading time segments for single device.""" mock_config_entry.add_to_hass(hass) @@ -45,10 +47,7 @@ async def test_read_time_segments_single_device( return_response=True, ) - assert response is not None - assert "time_segments" in response - assert isinstance(response["time_segments"], list) - assert len(response["time_segments"]) == 9 # Returns all 9 segments (1-9) + assert response == snapshot async def test_update_time_segment_charge_mode( From 1017d6fcd779d98b255af776009720abbaab1797 Mon Sep 17 00:00:00 2001 From: Joost Lekkerkerker Date: Tue, 16 Dec 2025 11:45:31 +0100 Subject: [PATCH 17/17] Update homeassistant/components/growatt_server/const.py --- homeassistant/components/growatt_server/const.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/homeassistant/components/growatt_server/const.py b/homeassistant/components/growatt_server/const.py index d5b3405d575221..65fdfb054176df 100644 --- a/homeassistant/components/growatt_server/const.py +++ b/homeassistant/components/growatt_server/const.py @@ -51,12 +51,3 @@ BATT_MODE_LOAD_FIRST = 0 BATT_MODE_BATTERY_FIRST = 1 BATT_MODE_GRID_FIRST = 2 - -BATT_MODE_MAP = { - "load-first": BATT_MODE_LOAD_FIRST, - "0": BATT_MODE_LOAD_FIRST, - "battery-first": BATT_MODE_BATTERY_FIRST, - "1": BATT_MODE_BATTERY_FIRST, - "grid-first": BATT_MODE_GRID_FIRST, - "2": BATT_MODE_GRID_FIRST, -}