Allow removing a Sofar battery pack that is no longer wired (#181283)

This commit is contained in:
darkrain-nl
2026-09-08 18:55:42 +02:00
committed by GitHub
parent 1a8ce697a7
commit d4842c8be3
6 changed files with 206 additions and 23 deletions
+50 -1
View File
@@ -2,6 +2,7 @@
from datetime import timedelta
import logging
from typing import TYPE_CHECKING
from modbus_connection import ModbusError, ModbusTcpParams
from sofar_modbus.modern.device import SofarInverter, identify
@@ -12,6 +13,7 @@ from homeassistant.components.sensor import (
SensorExtraStoredData,
SensorStateClass,
)
from homeassistant.config_entries import ConfigEntryState
from homeassistant.const import CONF_HOST, CONF_PORT, Platform
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryError
@@ -23,7 +25,13 @@ from homeassistant.helpers import (
)
from homeassistant.helpers.typing import ConfigType
from .const import CONF_UNIT_ID, DOMAIN, SCAN_INTERVAL, SETTINGS_SCAN_INTERVAL
from .const import (
BATTERY_COMPONENTS,
CONF_UNIT_ID,
DOMAIN,
SCAN_INTERVAL,
SETTINGS_SCAN_INTERVAL,
)
from .coordinator import SofarConfigEntry, SofarDataUpdateCoordinator, SofarRuntimeData
from .sensor import SENSOR_DESCRIPTIONS
from .services import async_setup_services
@@ -153,6 +161,47 @@ async def async_setup_entry(hass: HomeAssistant, entry: SofarConfigEntry) -> boo
return True
def _battery_pack_number(serial: str, identifier: str) -> int | None:
"""The battery pack a device identifier names, if it names one."""
prefix = f"{serial}_battery_"
if not identifier.startswith(prefix):
return None
suffix = identifier.removeprefix(prefix)
number = int(suffix) if suffix.isdecimal() else None
return number if number in BATTERY_COMPONENTS else None
async def async_remove_config_entry_device(
hass: HomeAssistant,
config_entry: SofarConfigEntry,
device_entry: dr.AnyDeviceEntry,
) -> bool:
"""Allow removing a battery pack the inverter no longer reports."""
serial = config_entry.unique_id
if TYPE_CHECKING:
assert serial is not None
runtime_data = (
config_entry.runtime_data
if config_entry.state is ConfigEntryState.LOADED
else None
)
packs: set[int] = set()
for domain, identifier in device_entry.identifiers:
if domain != DOMAIN:
continue
if identifier == serial or identifier.startswith(f"{serial}_pv_string_"):
return False
if (number := _battery_pack_number(serial, identifier)) is None:
continue
if runtime_data is not None and runtime_data.pack_is_wired(number):
return False
packs.add(number)
if runtime_data is not None:
runtime_data.wired_packs -= packs
return True
async def async_unload_entry(hass: HomeAssistant, entry: SofarConfigEntry) -> bool:
"""Unload a config entry."""
return await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
+4
View File
@@ -10,3 +10,7 @@ SCAN_INTERVAL = 5
SETTINGS_SCAN_INTERVAL = 60
CONF_UNIT_ID = "unit_id"
BATTERY_COMPONENTS = {
n: "battery_1_2" if n <= 2 else "battery_3_8" for n in range(1, 9)
}
+11 -2
View File
@@ -1,7 +1,7 @@
"""Data update coordinator for Sofar devices."""
from collections.abc import Awaitable, Callable
from dataclasses import dataclass
from dataclasses import dataclass, field
from datetime import timedelta
import logging
from typing import override
@@ -16,7 +16,7 @@ from homeassistant.core import HomeAssistant
from homeassistant.helpers import device_registry as dr
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
from .const import ATTR_MANUFACTURER, DOMAIN
from .const import ATTR_MANUFACTURER, BATTERY_COMPONENTS, DOMAIN
_LOGGER = logging.getLogger(__name__)
@@ -128,6 +128,7 @@ class SofarRuntimeData:
readings: SofarDataUpdateCoordinator
settings: SofarDataUpdateCoordinator
inverter_device_id: str
wired_packs: set[int] = field(default_factory=set)
@property
def served_components(self) -> frozenset[str]:
@@ -137,6 +138,14 @@ class SofarRuntimeData:
device.settings_components
)
def pack_is_wired(self, number: int) -> bool:
"""Whether a pack has answered, so it physically exists."""
component_name = BATTERY_COMPONENTS[number]
if component_name not in self.served_components:
return False
component = getattr(self.readings.device, component_name)
return bool(getattr(component, f"battery_voltage_{number}", None))
def coordinator_for(self, component: str) -> SofarDataUpdateCoordinator:
"""Which coordinator owns a given component's data."""
if component in self.readings.device.readings_components:
@@ -66,7 +66,7 @@ rules:
icon-translations: done
reconfiguration-flow: done
repair-issues: todo
stale-devices: todo
stale-devices: done
# Platinum
async-dependency: done
+5 -19
View File
@@ -6,7 +6,6 @@ from datetime import date
from enum import IntEnum
from typing import cast, override
from sofar_modbus.modern.device import SofarInverter
from sofar_modbus.modern.enums import FeedinLimitationMode, PassiveModeTimeoutAction
from homeassistant.components.sensor import (
@@ -32,6 +31,7 @@ from homeassistant.const import (
from homeassistant.core import HomeAssistant, callback
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
from .const import BATTERY_COMPONENTS
from .coordinator import SofarConfigEntry
from .entity import SofarEntity, SofarEntityDescription
@@ -46,7 +46,6 @@ async def async_setup_entry(
"""Set up the Sofar Inverter Modbus sensor platform."""
runtime_data = entry.runtime_data
served = runtime_data.served_components
device = runtime_data.readings.device
async_add_entities(
_sensor_class(description)(runtime_data, description)
@@ -54,15 +53,14 @@ async def async_setup_entry(
if description.component in served and not _is_battery_pack(description)
)
wired: set[int] = set()
@callback
def _async_add_wired_packs() -> None:
"""Add a pack's sensors the first time it reports a voltage."""
wired = runtime_data.wired_packs
new = {
number
for number in _BATTERY_COMPONENTS
if number not in wired and _pack_is_wired(device, served, number)
for number in BATTERY_COMPONENTS
if number not in wired and runtime_data.pack_is_wired(number)
}
if not new:
return
@@ -86,15 +84,6 @@ def _is_battery_pack(description: SofarSensorDescription) -> bool:
return description.part is not None and description.part[0] == "battery"
def _pack_is_wired(device: SofarInverter, served: frozenset[str], number: int) -> bool:
"""Whether a pack has answered, so it physically exists."""
component_name = _BATTERY_COMPONENTS[number]
if component_name not in served:
return False
component = getattr(device, component_name)
return bool(getattr(component, f"battery_voltage_{number}", None))
def _sensor_class(
description: SofarSensorDescription,
) -> type[SofarSensor | SofarTotalSensor]:
@@ -191,9 +180,6 @@ _PV_STRING_COMPONENTS = {
9: "pv_9_10",
10: "pv_9_10",
}
_BATTERY_COMPONENTS = {
n: "battery_1_2" if n <= 2 else "battery_3_8" for n in range(1, 9)
}
_PV_STRING_MEASUREMENTS = (
_PartMeasurement(
@@ -1516,4 +1502,4 @@ SENSOR_DESCRIPTIONS: tuple[SofarSensorDescription, ...] = (
SENSOR_DESCRIPTIONS += _part_sensors(
"pv_string", _PV_STRING_COMPONENTS, _PV_STRING_MEASUREMENTS
) + _part_sensors("battery", _BATTERY_COMPONENTS, _BATTERY_MEASUREMENTS)
) + _part_sensors("battery", BATTERY_COMPONENTS, _BATTERY_MEASUREMENTS)
+135
View File
@@ -19,6 +19,7 @@ from homeassistant.config_entries import ConfigEntryState
from homeassistant.const import STATE_UNAVAILABLE
from homeassistant.core import HomeAssistant
from homeassistant.helpers import device_registry as dr, entity_registry as er
from homeassistant.setup import async_setup_component
from . import (
MOCK_HW_VERSION,
@@ -31,6 +32,7 @@ from . import (
)
from tests.common import MockConfigEntry, async_fire_time_changed
from tests.typing import WebSocketGenerator
PV_POWER_REGISTER = 0x0586
BATTERY_3_VOLTAGE_REGISTER = 0x0612
@@ -577,3 +579,136 @@ async def test_battery_pack_appears_once_its_block_answers(
entity_id = entity_registry.async_get_entity_id(SENSOR_DOMAIN, DOMAIN, unique_id)
assert entity_id is not None
assert hass.states.get(entity_id).state == "51.5"
async def _setup_hybrid(
hass: HomeAssistant, connection: MockModbusConnection
) -> MockConfigEntry:
"""Set up a hybrid entry against a connection the caller still owns."""
entry = MockConfigEntry(
domain=DOMAIN,
unique_id=MOCK_HYBRID_SERIAL,
data=MOCK_USER_INPUT,
title=MOCK_HYBRID_MODEL,
)
entry.add_to_hass(hass)
with patch(
"homeassistant.components.sofar.async_get_unit",
side_effect=lambda hass, entry, params, unit_id: connection.for_unit(unit_id),
):
await hass.config_entries.async_setup(entry.entry_id)
await hass.async_block_till_done(wait_background_tasks=True)
return entry
@pytest.mark.parametrize(
("identifiers", "removable"),
[
pytest.param({(DOMAIN, MOCK_HYBRID_SERIAL)}, False, id="inverter"),
pytest.param(
{(DOMAIN, f"{MOCK_HYBRID_SERIAL}_battery_1")}, False, id="wired_pack"
),
pytest.param(
{(DOMAIN, f"{MOCK_HYBRID_SERIAL}_pv_string_2")}, False, id="pv_string"
),
pytest.param(
{(DOMAIN, f"{MOCK_HYBRID_SERIAL}_battery_9")}, True, id="pack_off_the_map"
),
pytest.param(
{(DOMAIN, f"{MOCK_HYBRID_SERIAL}_battery_x")}, True, id="unparseable_part"
),
pytest.param(
{(DOMAIN, f"{MOCK_HYBRID_SERIAL}_battery_\u00b2")},
True,
id="non_decimal_digit",
),
pytest.param(
{(DOMAIN, f"{MOCK_HYBRID_SERIAL}_gizmo_1")}, True, id="unknown_part_kind"
),
pytest.param({("other", MOCK_HYBRID_SERIAL)}, True, id="foreign_identifier"),
],
)
async def test_only_absent_battery_packs_can_be_removed(
hass: HomeAssistant,
hass_ws_client: WebSocketGenerator,
device_registry: dr.DeviceRegistry,
identifiers: set[tuple[str, str]],
removable: bool,
) -> None:
"""Test removal is refused for anything the inverter still reports."""
assert await async_setup_component(hass, "config", {})
connection = MockModbusConnection()
seed_hybrid_inverter(connection.for_unit(1))
entry = await _setup_hybrid(hass, connection)
device = device_registry.async_get_or_create(
config_entry_id=entry.entry_id, identifiers=identifiers
)
client = await hass_ws_client(hass)
response = await client.remove_device(device.id)
assert response["success"] is removable
assert (device_registry.async_get(device.id) is None) is removable
async def test_an_absent_pack_is_removable_while_the_entry_is_unloaded(
hass: HomeAssistant,
hass_ws_client: WebSocketGenerator,
device_registry: dr.DeviceRegistry,
) -> None:
"""Test a pack whose liveness cannot be checked is taken on trust."""
assert await async_setup_component(hass, "config", {})
connection = MockModbusConnection()
seed_hybrid_inverter(connection.for_unit(1))
entry = await _setup_hybrid(hass, connection)
await hass.config_entries.async_unload(entry.entry_id)
await hass.async_block_till_done()
device = device_registry.async_get_device_by_identifier(
(DOMAIN, f"{MOCK_HYBRID_SERIAL}_battery_1"), entry.entry_id
)
assert device is not None
client = await hass_ws_client(hass)
assert (await client.remove_device(device.id))["success"]
assert device_registry.async_get(device.id) is None
async def test_a_removed_pack_comes_back_without_a_restart(
hass: HomeAssistant,
freezer: FrozenDateTimeFactory,
hass_ws_client: WebSocketGenerator,
device_registry: dr.DeviceRegistry,
entity_registry: er.EntityRegistry,
) -> None:
"""Test removing a pack does not bar it from being added again."""
assert await async_setup_component(hass, "config", {})
connection = MockModbusConnection()
unit = connection.for_unit(1)
seed_hybrid_inverter(unit)
entry = await _setup_hybrid(hass, connection)
unit.holding[BATTERY_3_VOLTAGE_REGISTER] = 0
freezer.tick(timedelta(seconds=SCAN_INTERVAL))
async_fire_time_changed(hass)
await hass.async_block_till_done()
device = device_registry.async_get_device_by_identifier(
(DOMAIN, f"{MOCK_HYBRID_SERIAL}_battery_3"), entry.entry_id
)
assert device is not None
client = await hass_ws_client(hass)
assert (await client.remove_device(device.id))["success"]
unique_id = f"{MOCK_HYBRID_SERIAL}_battery_voltage_3"
assert entity_registry.async_get_entity_id(SENSOR_DOMAIN, DOMAIN, unique_id) is None
unit.holding[BATTERY_3_VOLTAGE_REGISTER] = 515
freezer.tick(timedelta(seconds=SCAN_INTERVAL))
async_fire_time_changed(hass)
await hass.async_block_till_done()
entity_id = entity_registry.async_get_entity_id(SENSOR_DOMAIN, DOMAIN, unique_id)
assert entity_id is not None
assert hass.states.get(entity_id).state == "51.5"