Skip to content

Commit 341bf7e

Browse files
0xAHAclaude
andcommitted
fix: an unknown profile key silently became a MIN 7-10kW
get_profile() falls back to min_7000_10000_tl_x for any key it does not recognise. That keeps setup alive, but it is a single-phase profile reading the 3000 range, so on any other model the integration loads cleanly and reports almost nothing - with no log line and no repair to trace it to. #360 hit it: a user hand-edited a profile into the component directory, updating replaced those files, and their entry still named the profile that had gone. The visible symptom was 'phase voltage and frequency show nothing', which points nowhere near the cause. Adds a warning in get_profile(), profile_exists() for callers that can surface it, and an unknown_profile repair issue naming the missing key and telling the user to re-select their model. The stale-entity cleanup is skipped in that state: the fallback's sensor set is not the device's, and treating it as authoritative would delete every entity the real profile created. Also fixes the placeholder test that should have caught the new repair being wrong. It compared strings.json against a hard-coded dict and searched only coordinator.py, so it verified nothing about a repair raised elsewhere. It now reads the actual translation_placeholders from every module that raises one, parsed with ast because the values are f-strings whose braces truncate any regex. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 2b28b04 commit 341bf7e

5 files changed

Lines changed: 123 additions & 15 deletions

File tree

custom_components/growatt_modbus/__init__.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,12 +376,42 @@ def _cleanup_unsupported_vpp_entities() -> None:
376376
# Not gated on connectivity, for the same reason as the blocks above: profile
377377
# membership is a static fact needing no inverter and no poll. Gating it is what
378378
# stopped the v1.4.0 cleanup running at all (#362).
379+
# A profile key that no longer exists resolves to min_7000_10000_tl_x, which loads
380+
# cleanly and reports almost nothing on any other model. Nothing fails, so there is
381+
# nothing for the user to search for — #360 spent a round trip on "my phase sensors
382+
# show nothing" that turned out to be this.
383+
#
384+
# Raised before the cleanup below, which is skipped in that state: the fallback's
385+
# sensor set is not this device's, and treating it as authoritative would delete
386+
# every entity the real profile had created.
387+
from .device_profiles import profile_exists
388+
389+
configured_profile = entry.data.get(CONF_INVERTER_SERIES, "")
390+
profile_is_known = profile_exists(configured_profile)
391+
if not profile_is_known:
392+
try:
393+
ir.async_create_issue(
394+
hass,
395+
DOMAIN,
396+
f"unknown_profile_{entry.entry_id}",
397+
is_fixable=False,
398+
severity=ir.IssueSeverity.ERROR,
399+
translation_key="unknown_profile",
400+
translation_placeholders={"profile": configured_profile or "(none)"},
401+
learn_more_url=(
402+
"https://github.com/0xAHA/Growatt_ModbusTCP/blob/main/"
403+
"docs/hardware/models.md"
404+
),
405+
)
406+
except Exception as err: # noqa: BLE001
407+
_LOGGER.debug("Could not create unknown-profile repair issue: %s", err)
408+
379409
# Imported here rather than at module scope: sensor.py imports coordinator.py, and
380410
# hoisting this creates a cycle at integration load.
381411
from .sensor import SENSOR_DEFINITIONS
382412
from .device_profiles import get_sensors_for_profile
383413

384-
profile_sensors = get_sensors_for_profile(entry.data.get(CONF_INVERTER_SERIES, ""))
414+
profile_sensors = get_sensors_for_profile(configured_profile) if profile_is_known else set()
385415
if profile_sensors:
386416
for sensor_key in SENSOR_DEFINITIONS:
387417
if sensor_key in profile_sensors:

custom_components/growatt_modbus/device_profiles.py

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
"""Device profiles for Growatt inverters."""
2+
import logging
23
from typing import Dict, Set
34

5+
_LOGGER = logging.getLogger(__name__)
6+
47
# ============================================================================
58
# SENSOR GROUPS
69
# ============================================================================
@@ -1136,9 +1139,42 @@ def resolve_profile_alias(series: str) -> str:
11361139
return PROFILE_ALIASES.get(series, series)
11371140

11381141

1142+
def profile_exists(series: str) -> bool:
1143+
"""Whether a profile key resolves to a real profile rather than the fallback.
1144+
1145+
Callers that can surface a problem to the user should check this first: get_profile()
1146+
cannot distinguish "you asked for MIN 7-10kW" from "you asked for something that does
1147+
not exist", because both return the same profile.
1148+
"""
1149+
return resolve_profile_alias(series) in INVERTER_PROFILES
1150+
1151+
11391152
def get_profile(series: str):
1140-
"""Get inverter profile by series name, resolving any alias first."""
1141-
return INVERTER_PROFILES.get(resolve_profile_alias(series), INVERTER_PROFILES["min_7000_10000_tl_x"])
1153+
"""Get inverter profile by series name, resolving any alias first.
1154+
1155+
An unknown key falls back to min_7000_10000_tl_x. That keeps setup alive rather than
1156+
raising, but it is a single-phase profile reading the 3000 range, so on anything else
1157+
it produces an integration that loads cleanly and reports almost nothing — with no
1158+
error to trace it to.
1159+
#360 hit this: a user hand-edited a profile into the component directory, and updating
1160+
the integration replaced those files. Their entry still named the vanished profile, so
1161+
they silently became a MIN.
1162+
1163+
The warning below is why this is not silent any more; __init__ also raises a repair
1164+
issue, because a log line alone does not reach anyone.
1165+
"""
1166+
resolved = resolve_profile_alias(series)
1167+
profile = INVERTER_PROFILES.get(resolved)
1168+
if profile is None:
1169+
_LOGGER.warning(
1170+
"Unknown inverter profile %r — falling back to 'min_7000_10000_tl_x'. "
1171+
"Sensors for your model will be missing or empty. Reconfigure the "
1172+
"integration and select your model. This usually means a hand-edited "
1173+
"profile was removed by an update.",
1174+
series,
1175+
)
1176+
return INVERTER_PROFILES["min_7000_10000_tl_x"]
1177+
return profile
11421178

11431179

11441180
def get_available_profiles(legacy_only: bool = False, friendly_names: bool = True) -> Dict[str, str]:

custom_components/growatt_modbus/strings.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@
9999
"title": "Growatt: settings are being reverted",
100100
"description": "Local changes to **{controls}** were reverted shortly after being set, so the inverter is not keeping them.\n\nThe usual causes, in order of likelihood:\n\n- **Growatt's cloud is pushing settings back.** A ShineWiFi or ShineLink dongle that is only uploading telemetry does not appear to be enough on its own — one user runs one alongside this integration with local writes persisting overnight. What overwrites local changes is the cloud sending settings *down*: remote control enabled in the ShinePhone app, or a schedule configured there. Turn those off before disconnecting anything.\n- **A prerequisite setting is not enabled.** On MOD inverters, *Allow Grid Charge* must be Enabled before time-of-use schedules will persist.\n- **The register is not writable on your firmware.** Some models accept the write and ignore it. The log has the register number if you want to report it.\n\nThis notice will not reappear until Home Assistant restarts."
101101
},
102+
"unknown_profile": {
103+
"title": "Growatt: inverter model no longer recognised",
104+
"description": "This entry is configured for the profile **{profile}**, which no longer exists in the integration.\n\nIt has fallen back to a MIN 7-10kW profile so Home Assistant can still start, but that profile reads different registers from your inverter. Most sensors will be missing or stuck at zero until this is corrected.\n\n**Fix it:** go to Settings > Devices & Services > Growatt Modbus > Configure, and select your model.\n\nThe usual cause is a profile added or edited by hand in the integration's folder, which an update then replaced. If you need a model the integration does not cover, please open an issue rather than editing the files - a profile added properly survives updates."
105+
},
102106
"gateway_malformed_frames": {
103107
"title": "Growatt: RS485 gateway returning mismatched responses",
104108
"description": "The gateway at **{gateway}** answered **{percent}%** of requests with a frame that did not match the request — typically a complete response to an *earlier* request, replayed.\n\n**Your data is not affected.** These responses are detected and discarded rather than decoded, which is what this check is for. But the reads are lost, so sensors update less often than they should.\n\nThis is a gateway problem, not an inverter problem. Things that have helped others:\n\n- Set **Max Register Block Size** to 25 in the integration options.\n- Check the gateway is in **Modbus TCP to RTU** mode, not transparent passthrough.\n- If it has a TCP timeout setting, make sure it is enabled rather than disabled, so dead sessions are cleaned up.\n\nSee the linked guide for gateways known to work and the settings they use."

custom_components/growatt_modbus/translations/en.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@
9999
"title": "Growatt: settings are being reverted",
100100
"description": "Local changes to **{controls}** were reverted shortly after being set, so the inverter is not keeping them.\n\nThe usual causes, in order of likelihood:\n\n- **Growatt's cloud is pushing settings back.** A ShineWiFi or ShineLink dongle that is only uploading telemetry does not appear to be enough on its own — one user runs one alongside this integration with local writes persisting overnight. What overwrites local changes is the cloud sending settings *down*: remote control enabled in the ShinePhone app, or a schedule configured there. Turn those off before disconnecting anything.\n- **A prerequisite setting is not enabled.** On MOD inverters, *Allow Grid Charge* must be Enabled before time-of-use schedules will persist.\n- **The register is not writable on your firmware.** Some models accept the write and ignore it. The log has the register number if you want to report it.\n\nThis notice will not reappear until Home Assistant restarts."
101101
},
102+
"unknown_profile": {
103+
"title": "Growatt: inverter model no longer recognised",
104+
"description": "This entry is configured for the profile **{profile}**, which no longer exists in the integration.\n\nIt has fallen back to a MIN 7-10kW profile so Home Assistant can still start, but that profile reads different registers from your inverter. Most sensors will be missing or stuck at zero until this is corrected.\n\n**Fix it:** go to Settings > Devices & Services > Growatt Modbus > Configure, and select your model.\n\nThe usual cause is a profile added or edited by hand in the integration's folder, which an update then replaced. If you need a model the integration does not cover, please open an issue rather than editing the files - a profile added properly survives updates."
105+
},
102106
"gateway_malformed_frames": {
103107
"title": "Growatt: RS485 gateway returning mismatched responses",
104108
"description": "The gateway at **{gateway}** answered **{percent}%** of requests with a frame that did not match the request — typically a complete response to an *earlier* request, replayed.\n\n**Your data is not affected.** These responses are detected and discarded rather than decoded, which is what this check is for. But the reads are lost, so sensors update less often than they should.\n\nThis is a gateway problem, not an inverter problem. Things that have helped others:\n\n- Set **Max Register Block Size** to 25 in the integration options.\n- Check the gateway is in **Modbus TCP to RTU** mode, not transparent passthrough.\n- If it has a TCP timeout setting, make sure it is enabled rather than disabled, so dead sessions are cleaned up.\n\nSee the linked guide for gateways known to work and the settings they use."

tests/test_gateway_health.py

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ def test_no_division_by_zero_on_a_fresh_connection():
9797
# still appears, so it looks like it works — it just says "gateway_malformed_frames" to
9898
# the user instead of explaining anything.
9999

100+
import ast
100101
import io
101102
import json
102103
import re
@@ -136,24 +137,57 @@ def test_repair_strings_have_title_and_description(filename):
136137
assert body.get("description"), f"{filename}: issue '{key}' has no description"
137138

138139

140+
def _supplied_placeholders() -> dict[str, set[str]]:
141+
"""Placeholder names each repair issue actually passes, read from the source.
142+
143+
Keyed by translation_key, taken from the `translation_placeholders={...}` dict in the
144+
same `ir.async_create_issue(...)` call. Repairs are raised from more than one module,
145+
so every file that creates them has to be searched — an earlier version of this test
146+
looked only in coordinator.py and compared against a hard-coded list, which meant it
147+
verified nothing about a repair raised anywhere else.
148+
149+
Parsed with `ast` rather than regex: the placeholder values are f-strings such as
150+
f"{hub.host}:{hub.port}", whose own braces terminate any non-greedy brace match and
151+
silently drop every key after the first.
152+
"""
153+
supplied: dict[str, set[str]] = {}
154+
for filename in ("coordinator.py", "__init__.py"):
155+
tree = ast.parse((COMPONENT / filename).read_text(encoding="utf-8"))
156+
for node in ast.walk(tree):
157+
if not isinstance(node, ast.Call):
158+
continue
159+
func = node.func
160+
if not (isinstance(func, ast.Attribute) and func.attr == "async_create_issue"):
161+
continue
162+
kwargs = {kw.arg: kw.value for kw in node.keywords if kw.arg}
163+
key_node = kwargs.get("translation_key")
164+
if not isinstance(key_node, ast.Constant):
165+
continue
166+
names: set[str] = set()
167+
ph = kwargs.get("translation_placeholders")
168+
if isinstance(ph, ast.Dict):
169+
names = {
170+
k.value for k in ph.keys
171+
if isinstance(k, ast.Constant) and isinstance(k.value, str)
172+
}
173+
supplied.setdefault(key_node.value, set()).update(names)
174+
return supplied
175+
176+
139177
def test_placeholders_used_in_strings_are_supplied_by_the_code():
140178
"""A placeholder with no matching value raises at render time, so the repair never
141179
appears — the failure is invisible rather than ugly."""
142180
data = json.load(io.open(COMPONENT / "strings.json", encoding="utf-8"))
143-
source = (COMPONENT / "coordinator.py").read_text(encoding="utf-8")
181+
supplied = _supplied_placeholders()
144182

145-
expected = {
146-
"write_reversion": {"controls"},
147-
"gateway_malformed_frames": {"gateway", "percent"},
148-
}
149183
for key, body in data.get("issues", {}).items():
150184
used = set(re.findall(r"\{([a-z_]+)\}", body["title"] + body["description"]))
151-
assert used == expected.get(key, set()), (
185+
assert key in supplied, (
186+
f"issue '{key}' is defined in strings.json but no ir.async_create_issue() "
187+
f"call raises it — either it is dead, or it is raised from a module this "
188+
f"test does not search"
189+
)
190+
assert used == supplied[key], (
152191
f"issue '{key}' uses placeholders {sorted(used)}; "
153-
f"the code supplies {sorted(expected.get(key, set()))}"
192+
f"the code supplies {sorted(supplied[key])}"
154193
)
155-
for placeholder in used:
156-
assert f'"{placeholder}"' in source, (
157-
f"issue '{key}' needs placeholder '{placeholder}' but coordinator.py "
158-
f"never supplies it"
159-
)

0 commit comments

Comments
 (0)