From 89d7f823a65a42668ca4adda0fe4a6ddaba8ffde Mon Sep 17 00:00:00 2001 From: Davide Bertola Date: Wed, 26 Aug 2026 17:13:20 +0200 Subject: [PATCH] Add battery charge current limit number entity to GoodWe --- homeassistant/components/goodwe/icons.json | 3 + homeassistant/components/goodwe/number.py | 56 ++++- homeassistant/components/goodwe/strings.json | 3 + tests/components/goodwe/test_number.py | 250 +++++++++++++++++++ 4 files changed, 306 insertions(+), 6 deletions(-) create mode 100644 tests/components/goodwe/test_number.py diff --git a/homeassistant/components/goodwe/icons.json b/homeassistant/components/goodwe/icons.json index 5f8ba77edecd82..3ba5171e18f4ce 100644 --- a/homeassistant/components/goodwe/icons.json +++ b/homeassistant/components/goodwe/icons.json @@ -6,6 +6,9 @@ } }, "number": { + "battery_charge_current": { + "default": "mdi:battery-charging" + }, "battery_discharge_depth": { "default": "mdi:battery-arrow-down" }, diff --git a/homeassistant/components/goodwe/number.py b/homeassistant/components/goodwe/number.py index 51b7729a4c8688..e874e9f7b1fe54 100644 --- a/homeassistant/components/goodwe/number.py +++ b/homeassistant/components/goodwe/number.py @@ -11,8 +11,14 @@ NumberDeviceClass, NumberEntity, NumberEntityDescription, + NumberMode, +) +from homeassistant.const import ( + PERCENTAGE, + EntityCategory, + UnitOfElectricCurrent, + UnitOfPower, ) -from homeassistant.const import PERCENTAGE, EntityCategory, UnitOfPower from homeassistant.core import HomeAssistant from homeassistant.helpers.device_registry import DeviceInfo from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback @@ -22,14 +28,24 @@ _LOGGER = logging.getLogger(__name__) +# GoodWe documents Modbus register 45353 (BattChargeCurrMax) as an unsigned +# 16-bit value with gain 10 and range [0, 1000], i.e. 0-100.0 A in 0.1 A steps. +# This is the range of the register itself, not a per-model hardware limit: +# the inverter and the battery BMS still enforce their own limits. +BATTERY_CHARGE_CURRENT_MAX = 100 + @dataclass(frozen=True, kw_only=True) class GoodweNumberEntityDescription(NumberEntityDescription): """Class describing Goodwe number entities.""" - getter: Callable[[Inverter], Awaitable[int]] - setter: Callable[[Inverter, int], Awaitable[None]] + getter: Callable[[Inverter], Awaitable[float]] + setter: Callable[[Inverter, float], Awaitable[None]] filter: Callable[[Inverter], bool] + # Converts the requested value to the value written to the inverter. + # Most settings are whole numbers, settings with a finer resolution + # (e.g. currents with 0.1 A steps) provide their own conversion. + converter: Callable[[float], float] = int def _get_setting_unit(inverter: Inverter, setting: str) -> str: @@ -37,6 +53,11 @@ def _get_setting_unit(inverter: Inverter, setting: str) -> str: return next((s.unit for s in inverter.settings() if s.id_ == setting), "") +def _has_setting(inverter: Inverter, setting: str) -> bool: + """Return whether the inverter advertises an inverter setting.""" + return any(s.id_ == setting for s in inverter.settings()) + + NUMBERS = ( # Only one of the export limits are added. # Availability is checked in the filter method. @@ -80,6 +101,24 @@ def _get_setting_unit(inverter: Inverter, setting: str) -> str: setter=lambda inv, val: inv.set_ongrid_battery_dod(val), filter=lambda inv: True, ), + # Battery charge current limit in A (inverter side, BMS limits still apply). + # Only added when the inverter advertises the setting. + GoodweNumberEntityDescription( + key="battery_charge_current", + translation_key="battery_charge_current", + entity_category=EntityCategory.CONFIG, + device_class=NumberDeviceClass.CURRENT, + native_unit_of_measurement=UnitOfElectricCurrent.AMPERE, + mode=NumberMode.BOX, + native_step=0.1, + native_min_value=0, + native_max_value=BATTERY_CHARGE_CURRENT_MAX, + getter=lambda inv: inv.read_setting("battery_charge_current"), + setter=lambda inv, val: inv.write_setting("battery_charge_current", val), + filter=lambda inv: _has_setting(inv, "battery_charge_current"), + # The register has a resolution of 0.1 A + converter=lambda val: round(val, 1), + ), ) @@ -101,6 +140,10 @@ async def async_setup_entry( # Inverter model does not support this setting _LOGGER.debug("Could not read inverter setting %s", description.key) continue + if current_value is None: + # Inverter rejected reading this setting + _LOGGER.debug("No value for inverter setting %s", description.key) + continue entities.append( InverterNumberEntity(device_info, description, inverter, current_value) @@ -121,7 +164,7 @@ def __init__( device_info: DeviceInfo, description: GoodweNumberEntityDescription, inverter: Inverter, - current_value: int, + current_value: float, ) -> None: """Initialize the number inverter setting entity.""" self.entity_description = description @@ -138,6 +181,7 @@ async def async_update(self) -> None: @override async def async_set_native_value(self, value: float) -> None: """Set new value.""" - await self.entity_description.setter(self._inverter, int(value)) - self._attr_native_value = value + native_value = self.entity_description.converter(value) + await self.entity_description.setter(self._inverter, native_value) + self._attr_native_value = float(native_value) self.async_write_ha_state() diff --git a/homeassistant/components/goodwe/strings.json b/homeassistant/components/goodwe/strings.json index fc8c3bccb2341d..e5ba1784b2eecb 100644 --- a/homeassistant/components/goodwe/strings.json +++ b/homeassistant/components/goodwe/strings.json @@ -24,6 +24,9 @@ } }, "number": { + "battery_charge_current": { + "name": "Battery charge current limit" + }, "battery_discharge_depth": { "name": "Depth of discharge (on-grid)" }, diff --git a/tests/components/goodwe/test_number.py b/tests/components/goodwe/test_number.py new file mode 100644 index 00000000000000..88299d4df86bf0 --- /dev/null +++ b/tests/components/goodwe/test_number.py @@ -0,0 +1,250 @@ +"""Test the GoodWe number platform.""" + +from typing import Any +from unittest.mock import AsyncMock, MagicMock, call + +from goodwe import InverterError, SensorKind +from goodwe.sensor import Current +import pytest + +from homeassistant.components.goodwe.const import ( + CONF_MODEL_FAMILY, + DEFAULT_NAME, + DOMAIN, +) +from homeassistant.components.number import ( + ATTR_MAX, + ATTR_MIN, + ATTR_MODE, + ATTR_STEP, + ATTR_VALUE, + DOMAIN as NUMBER_DOMAIN, + SERVICE_SET_VALUE, + NumberDeviceClass, + NumberMode, +) +from homeassistant.const import ( + ATTR_DEVICE_CLASS, + ATTR_ENTITY_ID, + ATTR_UNIT_OF_MEASUREMENT, + CONF_HOST, + CONF_PORT, + EntityCategory, + UnitOfElectricCurrent, +) +from homeassistant.core import HomeAssistant +from homeassistant.exceptions import ServiceValidationError +from homeassistant.helpers import entity_registry as er + +from .conftest import TEST_HOST, TEST_PORT, TEST_SERIAL + +from tests.common import MockConfigEntry + +BATTERY_CHARGE_CURRENT = "battery_charge_current" +BATTERY_CHARGE_CURRENT_ENTITY_ID = "number.goodwe_battery_charge_current_limit" +GRID_EXPORT_LIMIT_ENTITY_ID = "number.goodwe_grid_export_limit" + +# Setting definition as advertised by the goodwe library for ET inverters +BATTERY_CHARGE_CURRENT_SETTING = Current( + BATTERY_CHARGE_CURRENT, 45353, "Battery Charge Current", SensorKind.BAT +) + + +@pytest.fixture +def mock_config_entry() -> MockConfigEntry: + """Return a mocked GoodWe config entry.""" + return MockConfigEntry( + domain=DOMAIN, + title=DEFAULT_NAME, + version=2, + data={ + CONF_HOST: TEST_HOST, + CONF_PORT: TEST_PORT, + CONF_MODEL_FAMILY: "ET", + }, + unique_id=TEST_SERIAL, + ) + + +def configure_settings( + mock_inverter: MagicMock, settings: dict[str, Any], advertised: bool = True +) -> None: + """Configure the settings advertised and readable on the mocked inverter.""" + mock_inverter.settings.return_value = ( + (BATTERY_CHARGE_CURRENT_SETTING,) + if advertised and BATTERY_CHARGE_CURRENT in settings + else () + ) + + async def read_setting(setting_id: str) -> Any: + if setting_id not in settings: + raise ValueError(f'Unknown setting "{setting_id}"') + if isinstance(settings[setting_id], Exception): + raise settings[setting_id] + return settings[setting_id] + + mock_inverter.read_setting = AsyncMock(side_effect=read_setting) + mock_inverter.write_setting = AsyncMock() + + +async def setup_integration(hass: HomeAssistant, entry: MockConfigEntry) -> None: + """Set up the GoodWe integration.""" + entry.add_to_hass(hass) + assert await hass.config_entries.async_setup(entry.entry_id) + await hass.async_block_till_done() + + +async def test_battery_charge_current_limit( + hass: HomeAssistant, + entity_registry: er.EntityRegistry, + mock_inverter: MagicMock, + mock_config_entry: MockConfigEntry, +) -> None: + """Test the battery charge current limit entity is created from the setting.""" + configure_settings(mock_inverter, {BATTERY_CHARGE_CURRENT: 25.5}) + await setup_integration(hass, mock_config_entry) + + mock_inverter.read_setting.assert_any_call(BATTERY_CHARGE_CURRENT) + + state = hass.states.get(BATTERY_CHARGE_CURRENT_ENTITY_ID) + assert state is not None + assert state.state == "25.5" + assert state.attributes[ATTR_UNIT_OF_MEASUREMENT] == UnitOfElectricCurrent.AMPERE + assert state.attributes[ATTR_DEVICE_CLASS] == NumberDeviceClass.CURRENT + assert state.attributes[ATTR_MIN] == 0 + assert state.attributes[ATTR_MAX] == 100 + assert state.attributes[ATTR_STEP] == 0.1 + assert state.attributes[ATTR_MODE] == NumberMode.BOX + + entry = entity_registry.async_get(BATTERY_CHARGE_CURRENT_ENTITY_ID) + assert entry is not None + assert entry.unique_id == f"{DOMAIN}-{BATTERY_CHARGE_CURRENT}-{TEST_SERIAL}" + assert entry.entity_category is EntityCategory.CONFIG + assert entry.translation_key == BATTERY_CHARGE_CURRENT + + +async def test_battery_charge_current_limit_not_advertised( + hass: HomeAssistant, + mock_inverter: MagicMock, + mock_config_entry: MockConfigEntry, +) -> None: + """Test no entity is created and no read is attempted without the setting.""" + configure_settings(mock_inverter, {BATTERY_CHARGE_CURRENT: 25.5}, advertised=False) + await setup_integration(hass, mock_config_entry) + + assert hass.states.get(BATTERY_CHARGE_CURRENT_ENTITY_ID) is None + assert call(BATTERY_CHARGE_CURRENT) not in mock_inverter.read_setting.call_args_list + # Other number entities are not affected + assert hass.states.get(GRID_EXPORT_LIMIT_ENTITY_ID) is not None + + +@pytest.mark.parametrize( + "read_result", + [InverterError("Failed to read setting"), ValueError("Unknown setting"), None], + ids=["inverter_error", "value_error", "rejected"], +) +async def test_battery_charge_current_limit_read_error( + hass: HomeAssistant, + mock_inverter: MagicMock, + mock_config_entry: MockConfigEntry, + read_result: Exception | None, +) -> None: + """Test the entity is omitted when the advertised setting cannot be read.""" + configure_settings(mock_inverter, {BATTERY_CHARGE_CURRENT: read_result}) + await setup_integration(hass, mock_config_entry) + + mock_inverter.read_setting.assert_any_call(BATTERY_CHARGE_CURRENT) + assert hass.states.get(BATTERY_CHARGE_CURRENT_ENTITY_ID) is None + # Other number entities are not affected + assert hass.states.get(GRID_EXPORT_LIMIT_ENTITY_ID) is not None + + +@pytest.mark.parametrize( + ("requested_value", "written_value", "expected_state"), + [ + # Decimal values are written with their 0.1 A resolution + (20.3, 20.3, "20.3"), + (7, 7.0, "7.0"), + (0, 0.0, "0.0"), + (100, 100.0, "100.0"), + # Values are rounded to the 0.1 A resolution of the setting + (20.34, 20.3, "20.3"), + ], +) +async def test_set_battery_charge_current_limit( + hass: HomeAssistant, + mock_inverter: MagicMock, + mock_config_entry: MockConfigEntry, + requested_value: float, + written_value: float, + expected_state: str, +) -> None: + """Test setting the battery charge current limit writes the setting.""" + configure_settings(mock_inverter, {BATTERY_CHARGE_CURRENT: 25.5}) + await setup_integration(hass, mock_config_entry) + + await hass.services.async_call( + NUMBER_DOMAIN, + SERVICE_SET_VALUE, + { + ATTR_ENTITY_ID: BATTERY_CHARGE_CURRENT_ENTITY_ID, + ATTR_VALUE: requested_value, + }, + blocking=True, + ) + + mock_inverter.write_setting.assert_called_once_with( + BATTERY_CHARGE_CURRENT, written_value + ) + assert hass.states.get(BATTERY_CHARGE_CURRENT_ENTITY_ID).state == expected_state + + +@pytest.mark.parametrize("requested_value", [-0.1, 100.1]) +async def test_set_battery_charge_current_limit_out_of_range( + hass: HomeAssistant, + mock_inverter: MagicMock, + mock_config_entry: MockConfigEntry, + requested_value: float, +) -> None: + """Test out of range values are rejected without writing to the inverter.""" + configure_settings(mock_inverter, {BATTERY_CHARGE_CURRENT: 25.5}) + await setup_integration(hass, mock_config_entry) + + with pytest.raises(ServiceValidationError): + await hass.services.async_call( + NUMBER_DOMAIN, + SERVICE_SET_VALUE, + { + ATTR_ENTITY_ID: BATTERY_CHARGE_CURRENT_ENTITY_ID, + ATTR_VALUE: requested_value, + }, + blocking=True, + ) + + mock_inverter.write_setting.assert_not_called() + assert hass.states.get(BATTERY_CHARGE_CURRENT_ENTITY_ID).state == "25.5" + + +async def test_set_battery_charge_current_limit_write_error( + hass: HomeAssistant, + mock_inverter: MagicMock, + mock_config_entry: MockConfigEntry, +) -> None: + """Test the state is not updated when writing the setting fails.""" + configure_settings(mock_inverter, {BATTERY_CHARGE_CURRENT: 25.5}) + await setup_integration(hass, mock_config_entry) + mock_inverter.write_setting.side_effect = InverterError("Failed to write setting") + + with pytest.raises(InverterError): + await hass.services.async_call( + NUMBER_DOMAIN, + SERVICE_SET_VALUE, + { + ATTR_ENTITY_ID: BATTERY_CHARGE_CURRENT_ENTITY_ID, + ATTR_VALUE: 20.3, + }, + blocking=True, + ) + + mock_inverter.write_setting.assert_called_once_with(BATTERY_CHARGE_CURRENT, 20.3) + assert hass.states.get(BATTERY_CHARGE_CURRENT_ENTITY_ID).state == "25.5"