Skip to content

Commit 9e1a695

Browse files
0xAHAclaude
andcommitted
fix: sensors dropped from a profile lingered in the registry as unavailable
v1.5.3 removed dcdc_temp from the profiles that never had register 3176. The entities did not disappear - they stayed in the entity registry showing "unavailable", which is arguably worse than the 0.0 C it replaced: a dead entity reads as a broken sensor rather than one that was never meant to exist. Reported within minutes of the release by a MIN 10000TL-X owner. The cleanup for this already existed and did not cover it. It was a sequence of per-sensor blocks - one for the 1071/1091 SOC controls, one for battery_temp - so every removal needed someone to remember to add another, and this one did not get it. Replaced with the general rule: any sensor in SENSOR_DEFINITIONS that is not in the profile's sensor set is stale and gets removed. Safe as a blanket statement because sensor.py creates exactly the intersection of those two sets and nothing else, so anything outside it has no code path that could bring it back. Future removals now clean up after themselves. This is the second failure of the same cleanup. v1.4.0 gated it on the inverter being reachable, and coordinator.data is an empty GrowattData() during setup, so it never ran at all (#362). Both failure modes are covered by test_stale_entity_cleanup now: that the cleanup is driven by the profile rather than a hard-coded list, and that no connectivity guard wraps it. Both confirmed failing against the pre-fix source. Also records a field observation on MIN temperatures rather than acting on it. On a MIN 10000TL-X over months, 3093 == 3094 always and 3095 == 0 always - consistent with one sensor mirrored into Temp1/Temp2 and Temp3 unimplemented. All three addresses are correct per V1.39 and read independently; no fallback copies one into another. Left mapped deliberately: removing them would tidy one unit and silently delete working sensors on any MIN firmware that does populate them, and an entity reading zero is cheaper than one nobody can get back. The note exists so this is not re-litigated or "tidied" without a second device. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 0cb13f4 commit 9e1a695

5 files changed

Lines changed: 172 additions & 1 deletion

File tree

RELEASENOTES.md

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,30 @@
44

55
---
66

7+
## v1.5.4
8+
9+
Issues: #360, #362
10+
11+
- **Fix: sensors removed in v1.5.3 went `unavailable` instead of disappearing.**
12+
If v1.5.3 left you with a DC-DC Temperature entity showing *unavailable* rather than
13+
removing it, this is the follow-up. Upgrade and it will go.
14+
15+
Dropping a sensor from a profile stops it being created, but Home Assistant keeps what
16+
earlier versions already registered — so it lingers as a dead entity. Arguably worse
17+
than the 0.0 °C it replaced, because an unavailable sensor looks like something broken
18+
rather than something that was never meant to exist.
19+
20+
The cleanup that handles this existed, but was a list of specific sensor names, so each
21+
new removal needed its own entry and this one never got one. It is now driven by the
22+
profile itself: any sensor the profile does not list is removed, because nothing can
23+
recreate it. Future removals clean up after themselves.
24+
25+
This is the second time this cleanup has failed. In v1.4.0 it was gated on the inverter
26+
being reachable, which never holds during setup, so it did not run at all. Both failure
27+
modes are now covered by tests.
28+
29+
---
30+
731
## v1.5.3
832

933
Issues: #360, #362

custom_components/growatt_modbus/__init__.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,44 @@ def _cleanup_unsupported_vpp_entities() -> None:
359359
)
360360
entity_registry.async_remove(stale_eid)
361361

362+
# General rule: a sensor the current profile does not list cannot be recreated, so a
363+
# registry entry for it is stale by definition.
364+
#
365+
# The two blocks above each clean up one specific removal, which meant every future
366+
# removal needed its own block — and the next one didn't get it. v1.5.3 dropped
367+
# dcdc_temp from ~26 profiles that never had the register, and the entities did not
368+
# disappear: they sat in the registry showing "unavailable", which is arguably worse
369+
# than the 0.0 °C it replaced, because it looks like a broken sensor rather than one
370+
# that was never meant to exist.
371+
#
372+
# Safe as a blanket rule because sensor.py creates exactly
373+
# `SENSOR_DEFINITIONS ∩ get_sensors_for_profile(series)` and nothing else — anything
374+
# outside that intersection has no code path that could bring it back.
375+
#
376+
# Not gated on connectivity, for the same reason as the blocks above: profile
377+
# membership is a static fact needing no inverter and no poll. Gating it is what
378+
# stopped the v1.4.0 cleanup running at all (#362).
379+
# Imported here rather than at module scope: sensor.py imports coordinator.py, and
380+
# hoisting this creates a cycle at integration load.
381+
from .sensor import SENSOR_DEFINITIONS
382+
from .device_profiles import get_sensors_for_profile
383+
384+
profile_sensors = get_sensors_for_profile(entry.data.get(CONF_INVERTER_SERIES, ""))
385+
if profile_sensors:
386+
for sensor_key in SENSOR_DEFINITIONS:
387+
if sensor_key in profile_sensors:
388+
continue
389+
stale_eid = entity_registry.async_get_entity_id(
390+
"sensor", DOMAIN, f"{entry.entry_id}_{sensor_key}"
391+
)
392+
if stale_eid:
393+
_LOGGER.info(
394+
"Removing %s — '%s' is not in the %s profile, so nothing can "
395+
"populate it and it would linger as unavailable",
396+
stale_eid, sensor_key, entry.data.get(CONF_INVERTER_SERIES, "?"),
397+
)
398+
entity_registry.async_remove(stale_eid)
399+
362400
# Per-entry state lives on the entry itself. hass.data[DOMAIN] is now reserved
363401
# solely for "_connections", the cross-entry shared-connection registry.
364402
entry.runtime_data = coordinator

custom_components/growatt_modbus/manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@
1212
"pymodbus>=3.0.0",
1313
"pyserial>=3.4"
1414
],
15-
"version": "1.5.3"
15+
"version": "1.5.4"
1616
}

custom_components/growatt_modbus/profiles/min.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,23 @@
8787
3092: {'name': 'bus_voltage', 'scale': 0.1, 'unit': 'V', 'desc': 'DC bus voltage'},
8888

8989
# Temperatures
90+
#
91+
# All three addresses are correct per V1.39 (Temp1/Temp2/Temp3, 0.1 C) and are
92+
# read independently — no fallback copies one into another. What a MIN reports
93+
# into them is a separate question, and on a MIN 10000TL-X observed over months:
94+
#
95+
# 3093 == 3094 at all times, 3095 == 0 at all times
96+
#
97+
# consistent with one physical sensor mirrored into Temp1 and Temp2 with Temp3
98+
# unimplemented. On hardware that has three sensors they diverge — an SPA-TL3
99+
# scan read the equivalent block as 37.8 / 20.0 / 33.3 C (#360).
100+
#
101+
# DELIBERATELY LEFT MAPPED. Removing ipm_temp and boost_temp here would tidy up
102+
# a duplicate and an always-zero entity on that one unit, and silently delete
103+
# working sensors on any MIN firmware that does populate them. One device is not
104+
# enough to change a profile shared by every MIN, and an entity reading zero is
105+
# cheaper than a missing one nobody can get back. Revisit only with readings from
106+
# a second MIN.
90107
3093: {'name': 'inverter_temp', 'scale': 0.1, 'unit': '°C', 'desc': 'Inverter temperature', 'signed': True},
91108
3094: {'name': 'ipm_temp', 'scale': 0.1, 'unit': '°C', 'desc': 'IPM temperature', 'signed': True},
92109
3095: {'name': 'boost_temp', 'scale': 0.1, 'unit': '°C', 'desc': 'Boost temperature', 'signed': True},
@@ -236,6 +253,23 @@
236253
3092: {'name': 'bus_voltage', 'scale': 0.1, 'unit': 'V', 'desc': 'DC bus voltage'},
237254

238255
# Temperatures
256+
#
257+
# All three addresses are correct per V1.39 (Temp1/Temp2/Temp3, 0.1 C) and are
258+
# read independently — no fallback copies one into another. What a MIN reports
259+
# into them is a separate question, and on a MIN 10000TL-X observed over months:
260+
#
261+
# 3093 == 3094 at all times, 3095 == 0 at all times
262+
#
263+
# consistent with one physical sensor mirrored into Temp1 and Temp2 with Temp3
264+
# unimplemented. On hardware that has three sensors they diverge — an SPA-TL3
265+
# scan read the equivalent block as 37.8 / 20.0 / 33.3 C (#360).
266+
#
267+
# DELIBERATELY LEFT MAPPED. Removing ipm_temp and boost_temp here would tidy up
268+
# a duplicate and an always-zero entity on that one unit, and silently delete
269+
# working sensors on any MIN firmware that does populate them. One device is not
270+
# enough to change a profile shared by every MIN, and an entity reading zero is
271+
# cheaper than a missing one nobody can get back. Revisit only with readings from
272+
# a second MIN.
239273
3093: {'name': 'inverter_temp', 'scale': 0.1, 'unit': '°C', 'desc': 'Inverter temperature', 'signed': True},
240274
3094: {'name': 'ipm_temp', 'scale': 0.1, 'unit': '°C', 'desc': 'IPM temperature', 'signed': True},
241275
3095: {'name': 'boost_temp', 'scale': 0.1, 'unit': '°C', 'desc': 'Boost temperature', 'signed': True},

tests/test_stale_entity_cleanup.py

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
"""Sensors dropped from a profile must be removed from the entity registry.
2+
3+
Removing a sensor from a profile's set stops it being created. It does not remove what
4+
earlier versions already registered, so the entity stays in Home Assistant showing
5+
`unavailable` — which reads as a broken sensor rather than one that was never meant to
6+
exist, and is arguably worse than the wrong value it replaced.
7+
8+
This cleanup has now failed twice, each time differently:
9+
10+
- v1.4.0 gated it on the inverter being reachable. `async_config_entry_first_refresh`
11+
seeds an empty `GrowattData()`, so the guard was never satisfied during setup and the
12+
block never ran at all (#362).
13+
- v1.5.3 dropped `dcdc_temp` from ~26 profiles, but the cleanup was a per-sensor block
14+
listing specific names, and the new removal never got one. The entities went
15+
unavailable instead of disappearing.
16+
17+
The fix for the second was a general rule: anything in SENSOR_DEFINITIONS that is not in
18+
the profile's sensor set is stale, because sensor.py creates exactly the intersection of
19+
those two and nothing else. These tests hold both properties.
20+
21+
Parsed from source: __init__.py imports Home Assistant, which the HA-free suite cannot
22+
load.
23+
"""
24+
from __future__ import annotations
25+
26+
import ast
27+
import re
28+
from pathlib import Path
29+
30+
INIT = (Path(__file__).parent.parent / "custom_components" / "growatt_modbus"
31+
/ "__init__.py")
32+
SOURCE = INIT.read_text(encoding="utf-8")
33+
34+
35+
def _setup_entry_source() -> str:
36+
for node in ast.walk(ast.parse(SOURCE)):
37+
if isinstance(node, ast.AsyncFunctionDef) and node.name == "async_setup_entry":
38+
return ast.get_source_segment(SOURCE, node) or ""
39+
raise AssertionError("async_setup_entry not found")
40+
41+
42+
def test_cleanup_is_driven_by_the_profile_sensor_set():
43+
"""A per-sensor list only cleans up the removals someone remembered to add to it."""
44+
body = _setup_entry_source()
45+
assert "get_sensors_for_profile" in body, (
46+
"stale-entity cleanup does not consult the profile's sensor set, so it can only "
47+
"remove sensors that were hard-coded into it by name"
48+
)
49+
assert re.search(r"for sensor_key in SENSOR_DEFINITIONS", body), (
50+
"cleanup does not iterate SENSOR_DEFINITIONS, so a dropped sensor with no "
51+
"dedicated block will linger in the registry as unavailable"
52+
)
53+
54+
55+
def test_cleanup_actually_removes():
56+
body = _setup_entry_source()
57+
assert "async_remove" in body, "cleanup never calls async_remove"
58+
59+
60+
def test_cleanup_is_not_gated_on_connectivity():
61+
"""The v1.4.0 failure. Profile membership is static — it needs no inverter and no
62+
poll, and requiring one means the cleanup never runs during setup, which is the only
63+
time it matters.
64+
"""
65+
body = _setup_entry_source()
66+
# Find the general cleanup block and check no connectivity guard wraps it.
67+
idx = body.find("for sensor_key in SENSOR_DEFINITIONS")
68+
assert idx != -1, "general cleanup block not found"
69+
window = body[max(0, idx - 900):idx]
70+
for guard in ("serial_number", "has_real_data", "last_update_success"):
71+
assert guard not in window, (
72+
f"stale-entity cleanup appears gated on {guard!r}. coordinator.data is an "
73+
f"empty placeholder during setup, so such a guard stops the cleanup running "
74+
f"at all — this is the v1.4.0 regression (#362)."
75+
)

0 commit comments

Comments
 (0)