Skip to content

Commit 1576a32

Browse files
0xAHAclaude
andcommitted
Bump version to v1.3.0 — Integration Quality Improvements
Phases 0-1 of the Bronze/Silver compliance plan. No user-facing behaviour changes. DIAGNOSTICS PLATFORM (new diagnostics.py) One-click JSON dump from the integration and device pages: entry data and options, selected profile and register map, coordinator health (online state, consecutive failures, slow-poll mode), client state (backoff, block-size override, suppressed optional ranges), shared-connection state and refcount, and the current decoded GrowattData. Host, device path and serial are redacted via async_redact_data. Deliberately not a replacement for the register scanner — they answer different questions. Diagnostics reports integration state and works when every read is failing; the scanner probes what the hardware responds to, including registers outside the selected profile. That distinction is documented in the module docstring, because the scanner is the tool that found 31059 and the TL-XH2 VPP-only layout, and neither is visible to diagnostics. Every attribute is read via getattr with a default, and _safe() coerces values, so a dump cannot raise — it is needed precisely when things are already broken. TEST SUITE (188 tests, ~0.4s, CI on push and PR) tests/conftest.py stubs the single unused homeassistant import in growatt_modbus.py and binds the component directory as a synthetic package, so the protocol layer is testable without installing HA. It deliberately does not execute the integration's __init__.py. test_register_decoding.py 13 scale, signedness, 32-bit pairing, combined_scale; the #361 signed-overflow case asserted in both directions; missing-register-is-None (#357) test_range_selection.py 10 the "is this range fatal?" rule that was wrong in #357, #361 and #364, plus shipped-profile checks test_status_codes.py 18 table selection per family; #348 and #363 test_profile_integrity.py 133 dropdown reachability (#360, #361), register map resolution, pair symmetry, name uniqueness test_connection_recovery.py 11 PR #365 transport-vs-protocol split and per-poll recovery budget, via a fake transport test_sensor_integrity.py 3 pre-existing Writing these found four pre-existing profile issues, none fixed here: MID_15000_25000TL3_X_V201 31222 declares pair=31223 without reciprocation MIN_TL_XH_3000_10000_V201 3000-range and VPP duplicates (3000 wins by insertion order — the reason TL-XH2 needed its own profile) SPH_3000_6000/7000_10000_V201 power_to_user R-phase and Total share names; identical on single-phase SPH_8000_10000_HU BMS registers 1086-1089 share names with 1013/1014/1040 and appear to be shadowed — the BMS values are the reason that profile exists All allowlisted with explanations so they cannot grow silently. Not fixed because changing which register feeds a sensor alters what users see and needs a field report first — the mistake that caused the v1.1.3 SPH regression. The SPH HU case is flagged in the release notes with a request for owners to check. CI now installs pymodbus and pyserial so the suite mirrors production, and the workflow is renamed from "Sensor Integrity Tests" since it is no longer only that. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent f05fc7e commit 1576a32

11 files changed

Lines changed: 1095 additions & 6 deletions

File tree

.github/workflows/tests.yaml

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
name: Sensor Integrity Tests
1+
name: Tests
22

33
on:
44
push:
@@ -17,8 +17,11 @@ jobs:
1717
with:
1818
python-version: "3.12"
1919

20-
- name: Install pytest
21-
run: pip install pytest
20+
# Home Assistant is deliberately NOT installed. The protocol layer is
21+
# HA-free, and tests/conftest.py stubs the one unused import so it stays
22+
# that way — keeping the suite fast and the coupling visible.
23+
- name: Install dependencies
24+
run: pip install pytest pymodbus pyserial
2225

23-
- name: Run sensor integrity tests
26+
- name: Run tests
2427
run: pytest tests/ -v

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# Growatt Modbus Integration for Home Assistant ☀️
44

55
![HACS Badge](https://img.shields.io/badge/HACS-Custom-orange.svg)
6-
![Version](https://img.shields.io/badge/Version-1.2.3-blue.svg)
6+
![Version](https://img.shields.io/badge/Version-1.3.0-blue.svg)
77
[![GitHub Issues](https://img.shields.io/github/issues/0xAHA/Growatt_ModbusTCP.svg)](https://github.com/0xAHA/Growatt_ModbusTCP/issues)
88
[![GitHub Stars](https://img.shields.io/github/stars/0xAHA/Growatt_ModbusTCP.svg?style=social)](https://github.com/0xAHA/Growatt_ModbusTCP)
99

RELEASENOTES.md

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

55
---
66

7+
## v1.3.0 — Integration Quality Improvements
8+
9+
No user-facing behaviour changes. This release adds a diagnostics download and a test
10+
suite covering the logic that has caused the most regressions.
11+
12+
- **New: Download Diagnostics**
13+
14+
*Settings → Devices & Services → Growatt Modbus → ⋮ → Download diagnostics*
15+
16+
One click produces a JSON file with the config entry and options, the selected
17+
profile and register map, coordinator health (online state, consecutive failures,
18+
whether slow-poll mode is active), client state (backoff, block size, suppressed
19+
ranges), shared-connection state, and the current decoded values. Host, device path
20+
and serial number are redacted automatically.
21+
22+
This does **not** replace the Universal Register Scanner. They answer different
23+
questions: diagnostics reports what the integration currently *thinks*, and works
24+
even when every read is failing; the scanner probes what the hardware actually
25+
responds to, including registers outside the selected profile. Ask for diagnostics
26+
first, and a scan when register discovery is needed.
27+
28+
- **New: 188-test suite**, run in CI on every push and pull request.
29+
30+
Every case corresponds to a bug that reached users:
31+
32+
| Area | Guards against |
33+
|---|---|
34+
| Register decoding | v1.2.1 AC power reported as 429,496,471 W — a signed 32-bit value read unsigned (#361) |
35+
| | Missing registers decoding as `0` instead of `None`, which made a dead link look like a healthy inverter (#357) |
36+
| Range selection | The three separate ways "is this range fatal on failure?" has been wrong — #357, #361, #364 |
37+
| Status codes | SPH rendering "Unknown (6)" after being moved off the hybrid table without field confirmation (#363) |
38+
| | MOD/WIT/TL-XH showing "Self-Test" during normal operation (#348) |
39+
| Profile registry | Profiles selectable by auto-detection but unrenderable in the options flow, which locked users out of every setting (#360, #361) |
40+
| | Asymmetric 32-bit pairs and duplicate register names |
41+
| Connection recovery | The transport-vs-protocol distinction and per-poll budget from PR #365 (#364) |
42+
43+
Home Assistant is not a test dependency — the protocol layer is HA-free, and the
44+
suite runs in well under a second.
45+
46+
- **Recorded, not fixed: four pre-existing profile issues** found by the new tests.
47+
They are allowlisted with explanations so they cannot grow silently, but changing
48+
which register feeds a sensor alters what users see and needs a field report first.
49+
50+
The one worth attention: on **SPH/SPM HU**, the BMS registers at 1086-1089
51+
(`battery_soc`, `battery_voltage`, `battery_temp`) share names with the base
52+
profile's 1013/1014/1040. Register lookup returns the first match and the base
53+
block is spread first, so the BMS values — described in the profile as the *actual*
54+
battery state of charge, and the reason the HU profile exists — appear to be
55+
unreachable. **If you run an SPH/SPM HU, please check whether your battery SOC and
56+
voltage match your BMS**, and open an issue either way.
57+
58+
---
59+
760
## v1.2.3
861

962
Issues: #361
Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,153 @@
1+
"""Diagnostics support for Growatt Modbus.
2+
3+
Surfaces a one-click state dump from the integration and device pages, so the
4+
questions that dominate issue triage — which profile is selected, which version is
5+
running, what the options are, whether the coordinator thinks it is online — can be
6+
answered without a round trip.
7+
8+
This is deliberately NOT a replacement for the Universal Register Scanner service.
9+
The two answer different questions:
10+
11+
diagnostics (here) What is the integration's current state?
12+
Works even when every read is failing, which is exactly when
13+
it is needed. Cannot see registers the profile does not define.
14+
15+
register scanner What does the hardware actually respond to?
16+
Probes ranges outside the selected profile, offers decode
17+
candidates per register, and reports per-register error text.
18+
That is how 31059 (total PV power) and the TL-XH2 VPP-only
19+
layout were found — neither is visible to diagnostics.
20+
21+
Ask for diagnostics first; ask for a scan when register discovery is needed.
22+
"""
23+
from __future__ import annotations
24+
25+
from dataclasses import asdict, is_dataclass
26+
from typing import Any
27+
28+
from homeassistant.components.diagnostics import async_redact_data
29+
from homeassistant.config_entries import ConfigEntry
30+
from homeassistant.core import HomeAssistant
31+
32+
from .const import DOMAIN
33+
34+
# Host/device path can identify a network or a person's hardware layout, and the
35+
# serial number identifies the unit itself. Users routinely paste diagnostics into
36+
# public issues, so redact by default rather than relying on them to remember.
37+
TO_REDACT = {
38+
"host",
39+
"device_path",
40+
"serial_number",
41+
"unique_id",
42+
}
43+
44+
45+
def _safe(value: Any) -> Any:
46+
"""Coerce a value into something JSON-serialisable.
47+
48+
Diagnostics must never raise — a dump that fails is worse than one with a gap,
49+
because it fails precisely when the integration is already misbehaving.
50+
"""
51+
if value is None or isinstance(value, (bool, int, float, str)):
52+
return value
53+
if isinstance(value, dict):
54+
return {str(k): _safe(v) for k, v in value.items()}
55+
if isinstance(value, (list, tuple, set)):
56+
return [_safe(v) for v in value]
57+
return str(value)
58+
59+
60+
async def async_get_config_entry_diagnostics(
61+
hass: HomeAssistant, entry: ConfigEntry
62+
) -> dict[str, Any]:
63+
"""Return diagnostics for a config entry."""
64+
coordinator = hass.data.get(DOMAIN, {}).get(entry.entry_id)
65+
66+
diagnostics: dict[str, Any] = {
67+
"entry": {
68+
"title": entry.title,
69+
"version": entry.version,
70+
"source": entry.source,
71+
"state": str(entry.state),
72+
"data": _safe(dict(entry.data)),
73+
"options": _safe(dict(entry.options)),
74+
},
75+
}
76+
77+
if coordinator is None:
78+
# Entry not loaded — still worth returning what we have. This is a state a
79+
# user can genuinely be in (failed setup, disabled entry) and the entry data
80+
# alone answers "which profile" and "what options".
81+
diagnostics["coordinator"] = None
82+
diagnostics["note"] = (
83+
"Coordinator not loaded — the entry may be disabled, or setup may have "
84+
"failed. Entry data and options above are still accurate."
85+
)
86+
return async_redact_data(diagnostics, TO_REDACT)
87+
88+
client = getattr(coordinator, "_client", None)
89+
hub = getattr(coordinator, "_hub", None)
90+
91+
diagnostics["coordinator"] = {
92+
# Health — the first thing to look at when someone reports "unavailable"
93+
# or "stuck at zero".
94+
"inverter_online": getattr(coordinator, "_inverter_online", None),
95+
"ever_had_real_data": getattr(coordinator, "_ever_had_real_data", None),
96+
"last_update_success": getattr(coordinator, "last_update_success", None),
97+
"consecutive_failures": getattr(coordinator, "_consecutive_failures", None),
98+
"failure_threshold": getattr(coordinator, "_failure_threshold", None),
99+
"update_interval": _safe(getattr(coordinator, "update_interval", None)),
100+
"normal_update_interval": _safe(getattr(coordinator, "_normal_update_interval", None)),
101+
"offline_update_interval": _safe(getattr(coordinator, "_offline_update_interval", None)),
102+
"in_slow_poll_mode": (
103+
getattr(coordinator, "update_interval", None)
104+
== getattr(coordinator, "_offline_update_interval", None)
105+
),
106+
# Identity
107+
"register_map_key": getattr(coordinator, "_register_map_key", None),
108+
"serial_number": getattr(coordinator, "_serial_number", None),
109+
"firmware_version": getattr(coordinator, "_firmware_version", None),
110+
# Energy-guard state — relevant to every "my totals look wrong" report
111+
"midnight_grace_expires": _safe(getattr(coordinator, "_midnight_grace_expires", None)),
112+
"retained_daily_totals": _safe(getattr(coordinator, "_retained_daily_totals", None)),
113+
"retained_lifetime_totals": _safe(getattr(coordinator, "_retained_lifetime_totals", None)),
114+
"pending_write_checks": _safe(getattr(coordinator, "_pending_write_checks", None)),
115+
}
116+
117+
if client is not None:
118+
diagnostics["client"] = {
119+
"connection_type": getattr(client, "connection_type", None),
120+
"slave_id": getattr(client, "slave_id", None),
121+
"backed_off": getattr(client, "_backed_off", None),
122+
"consecutive_read_failures": getattr(client, "_consecutive_read_failures", None),
123+
"min_read_interval": getattr(client, "min_read_interval", None),
124+
"block_size_override": getattr(client, "_block_size_override", None),
125+
"profile_max_block_size": _safe(
126+
(getattr(client, "register_map", None) or {}).get("max_block_size")
127+
),
128+
# Ranges suppressed after repeated failure — explains "why is this
129+
# sensor empty" without needing a scan.
130+
"failed_optional_ranges": _safe(getattr(client, "_failed_optional_ranges", None)),
131+
}
132+
133+
if hub is not None:
134+
diagnostics["shared_connection"] = {
135+
"active": True,
136+
"port": getattr(hub, "port", None),
137+
"refcount": getattr(hub, "_refcount", None),
138+
"connected": getattr(hub, "_connected", None),
139+
"recoveries_this_poll": getattr(hub, "_recoveries_this_poll", None),
140+
"max_recoveries_per_poll": getattr(hub, "_max_recoveries_per_poll", None),
141+
}
142+
else:
143+
diagnostics["shared_connection"] = {"active": False}
144+
145+
# Current decoded values. Shows at a glance which sensor groups are populated
146+
# and which are flat zero — the signature of a failed or unsupported range.
147+
data = getattr(coordinator, "data", None)
148+
if data is not None and is_dataclass(data):
149+
diagnostics["data"] = _safe(asdict(data))
150+
else:
151+
diagnostics["data"] = None
152+
153+
return async_redact_data(diagnostics, TO_REDACT)

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.2.3"
15+
"version": "1.3.0"
1616
}

tests/conftest.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
"""Test configuration.
2+
3+
Home Assistant is not a test dependency, and it does not need to be: the protocol
4+
layer is almost entirely HA-free already. `growatt_modbus.py` imports
5+
`homeassistant.config_entries.ConfigEntry` at module scope but never uses it, and
6+
`const.py` / `profiles/` import nothing outside the standard library.
7+
8+
Two pieces of setup make that layer directly testable:
9+
10+
1. A minimal `homeassistant` stub, purely to satisfy the dead import above. If that
11+
import is ever removed this stub becomes a no-op and can go with it.
12+
13+
2. A synthetic package rooted at the component directory. The modules use relative
14+
imports (`from .const import ...`), so they need a package context — but the real
15+
`__init__.py` pulls in the coordinator and the HA entity stack. Binding a bare
16+
package object to the component directory gives relative imports somewhere to
17+
resolve without executing the integration's setup code.
18+
"""
19+
from __future__ import annotations
20+
21+
import importlib
22+
import sys
23+
import types
24+
from pathlib import Path
25+
26+
REPO_ROOT = Path(__file__).parent.parent
27+
COMPONENT_DIR = REPO_ROOT / "custom_components" / "growatt_modbus"
28+
29+
PKG = "growatt_under_test"
30+
31+
32+
def _stub_homeassistant() -> None:
33+
if "homeassistant" in sys.modules:
34+
return
35+
ha = types.ModuleType("homeassistant")
36+
config_entries = types.ModuleType("homeassistant.config_entries")
37+
38+
class ConfigEntry: # noqa: D401 - stand-in for an unused annotation
39+
"""Placeholder; never exercised by these tests."""
40+
41+
config_entries.ConfigEntry = ConfigEntry
42+
ha.config_entries = config_entries
43+
sys.modules["homeassistant"] = ha
44+
sys.modules["homeassistant.config_entries"] = config_entries
45+
46+
47+
def _bind_component_package() -> None:
48+
"""Expose the component directory as an importable package.
49+
50+
Deliberately does NOT execute custom_components/growatt_modbus/__init__.py —
51+
that file wires up the HA integration. We only want the protocol modules.
52+
"""
53+
if PKG in sys.modules:
54+
return
55+
pkg = types.ModuleType(PKG)
56+
pkg.__path__ = [str(COMPONENT_DIR)]
57+
sys.modules[PKG] = pkg
58+
59+
60+
_stub_homeassistant()
61+
_bind_component_package()
62+
63+
64+
def component(module: str):
65+
"""Import a component module by name, e.g. component('growatt_modbus')."""
66+
return importlib.import_module(f"{PKG}.{module}")

0 commit comments

Comments
 (0)