Skip to content

Commit dc18796

Browse files
0xAHAclaude
andcommitted
fix: dcdc_temp published 0.0 on every model without register 3176 (#360, #362)
A regression we shipped in v1.4.0, while fixing this exact class of bug. 7a0d194 identified register 3176 on MOD/MID as the DC-DC stage rather than battery temperature, and added "dcdc_temp" to the shared TEMPERATURE_SENSORS group with the note "only appears where a profile actually defines the register, so this is a no-op elsewhere". That is the belief device_profiles.py exists to warn against: membership of a profile's sensor set is what creates the entity, and the register only decides whether it ever gets a value. Three register maps define dcdc_temp. Twenty-six profiles included the group. Every one of those gained a DC-DC Temperature entity reporting freezing point forever. Inverted to opt-in via DCDC_TEMP_SENSOR, applied to the five profiles whose maps define the register. A sensor group shared by everything is the wrong home for a sensor only a few models can populate: adding to it is silent, and the failure is invisible because nothing errors. Found while auditing the same fault on SPA-TL3, prompted by the #360 scan CSV. That scan also settled two things guessed at in v1.5.2: - registers 94/95 answer 20.0 C and 33.3 C alongside 93 at 37.8 C, so SPH-TL3's ipm_temp and boost_temp were phantoms fixable by mapping rather than removal. Both profiles sharing that map have been publishing 0.0 for them. - input 53/54 = 0/20 and 55/56 = 0/23139 -> 2.0 kWh today, 2313.9 kWh total, with the SPA extended block agreeing at 2053-2056. ENERGY_SENSORS was excluded from SPA-TL3 on the assumption those count PV generation; they count inverter output, which a discharging battery produces. Restored. Worth recording: the reporter checked the same registers and said there was no real data, because 53 and 55 are the HIGH words and read zero. The file settled in seconds what two rounds of asking had not. test_phantom_temperatures.py fails if a temperature sensor is declared without a register behind it. Six profiles have pre-existing phantoms with no scan to resolve them - listed explicitly so the count can only go down, and the list is itself checked so a name cannot linger there after being fixed. Scoped to temperatures on purpose. The same check across all sensors is mostly false positives, because status, last_update and the grid import/export pair are computed in the coordinator rather than read from an address. A noisy test gets ignored. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 1247c0c commit dc18796

5 files changed

Lines changed: 190 additions & 17 deletions

File tree

RELEASENOTES.md

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

55
---
66

7+
## v1.5.3
8+
9+
Issues: #360, #362
10+
11+
- **Fix: a "DC-DC Temperature" sensor reporting 0.0 °C on models that have no such
12+
sensor.** Introduced by us in v1.4.0, and it is the same bug that release was fixing.
13+
14+
v1.4.0 identified register 3176 on MOD/MID as the DC-DC stage rather than battery
15+
temperature, and added `dcdc_temp` to the shared temperature sensor group — a group
16+
almost every profile includes. Only MOD/MID, SPF and SPE define that register, so every
17+
other model gained an entity that could never have a value and published freezing point
18+
instead.
19+
20+
A sensor is created because it is in a profile's set; the register only decides whether
21+
it has a value. The note added at the time said the opposite. It now belongs to the
22+
profiles that can populate it, and a test fails if a temperature sensor is ever declared
23+
without a register behind it.
24+
25+
**If you saw a DC-DC Temperature entity stuck at 0.0 °C, it will disappear on upgrade.**
26+
MOD, MID, SPF and SPE keep theirs — those are real readings.
27+
28+
- **SPH-TL3 and SPA-TL3 gain real IPM and Boost temperatures.** Both sensors were declared
29+
but had no registers, so both reported 0.0 °C. A full scan of the #360 device answered
30+
registers 94 and 95 with 20.0 °C and 33.3 °C alongside the inverter temperature at
31+
37.8 °C, so these are now mapped rather than removed.
32+
33+
- **SPA-TL3 regains Energy Today and Energy Total.** v1.5.2 excluded them, assuming they
34+
counted solar generation an AC-coupled inverter does not have. The same scan shows
35+
2.0 kWh today and 2313.9 kWh total, with the SPA extended block agreeing at a second
36+
address. They measure what the inverter puts out, and a discharging battery produces
37+
output like anything else.
38+
39+
---
40+
741
## v1.5.2
842

943
Issues: #360, #362

custom_components/growatt_modbus/device_profiles.py

Lines changed: 47 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,20 @@
9797
# hard filter available, so exclusion has to happen here.
9898
NO_BATTERY_TEMP: Set[str] = {"battery_temp"}
9999

100-
# Same mechanism, different sensor. dcdc_temp reads register 3176 (Bdc1Temp1) on MOD/MID
101-
# and has no equivalent anywhere in the SPA ranges, so a profile that wants the ordinary
102-
# inverter/IPM/boost temperatures has to subtract it or ship a permanent 0.0 °C.
103-
NO_DCDC_TEMP: Set[str] = {"dcdc_temp"}
100+
# dcdc_temp is NOT in TEMPERATURE_SENSORS, and must not be put back there.
101+
#
102+
# It was, briefly, and the consequences were the exact bug the change above exists to
103+
# prevent. The #362 fix identified register 3176 on MOD/MID as the DC-DC stage and added
104+
# "dcdc_temp" to the shared TEMPERATURE_SENSORS group — which almost every profile
105+
# includes. Only three register maps define the register at all, so every other model
106+
# gained a "DC-DC Temperature" entity reporting 0.0 °C: a phantom created while fixing a
107+
# phantom, verified on the two profiles that have the register and on none of the
108+
# twenty-six that don't.
109+
#
110+
# Opt in per profile instead. A sensor group shared by everything is the wrong home for
111+
# a sensor only a few models can populate, because adding to it is silent and the failure
112+
# is invisible — nothing errors, an entity simply reports freezing forever.
113+
DCDC_TEMP_SENSOR: Set[str] = {"dcdc_temp"}
104114

105115
BMS_SENSORS: Set[str] = {
106116
"bms_status", "bms_error", "bms_warn_info",
@@ -112,10 +122,11 @@
112122

113123
TEMPERATURE_SENSORS: Set[str] = {
114124
"inverter_temp", "ipm_temp", "boost_temp",
115-
# MOD/MID expose Bdc1Temp1 at register 3176 — the battery-side DC-DC stage. It was
116-
# mapped as battery_temp until #362 identified it against ShineApp. Only appears
117-
# where a profile actually defines the register, so this is a no-op elsewhere.
118-
"dcdc_temp",
125+
# dcdc_temp deliberately absent — see DCDC_TEMP_SENSOR below. It was added here with
126+
# the note "only appears where a profile actually defines the register, so this is a
127+
# no-op elsewhere", which is the belief this whole file warns against: membership of
128+
# the sensor set is what creates the entity, and the register only decides whether it
129+
# has a value. It was not a no-op; it put 0.0 °C on twenty-six profiles.
119130
}
120131

121132
STATUS_SENSORS: Set[str] = {
@@ -613,6 +624,11 @@
613624
# SPH TL3 SERIES - Hybrid Storage (Three Phase with Battery)
614625
# ========================================================================
615626

627+
# dcdc_temp is subtracted from both: it reads register 3176 (Bdc1Temp1), which
628+
# exists on MOD/MID and nowhere in the SPH-TL3 map. It was in the sensor set anyway,
629+
# so every SPH-TL3 install has been publishing a DC-DC temperature of 0.0 °C. Found
630+
# while checking the same class of fault on SPA-TL3 (#360); the other two phantom
631+
# temperatures here were fixable by adding registers 94/95, this one is not.
616632
"sph_tl3_3000_10000": {
617633
"name": "SPH-TL3 Series 3000-10000",
618634
"description": "Hybrid 3-phase inverter with battery storage (3-10kW)",
@@ -667,7 +683,7 @@
667683
# GrowattData field, so a hasattr() gate cannot suppress it — the sensor
668684
# would be created and publish 0.0 degrees forever. That is the v1.4.1
669685
# battery-temp bug exactly, and the sensor set is the only hard filter.
670-
(TEMPERATURE_SENSORS - NO_DCDC_TEMP) |
686+
TEMPERATURE_SENSORS |
671687
# Every BMS sensor is gated on hasattr(), so only the four registers this
672688
# profile actually defines (1083/1085/1095/1096) create entities (#360).
673689
BMS_SENSORS |
@@ -688,10 +704,21 @@
688704
# entry — the alternative considered was renaming the SPH-TL3 option to mention SPA,
689705
# which would have made phantom PV entities the documented behaviour.
690706
#
691-
# ENERGY_SENSORS (energy_today/energy_total) is excluded for the same reason it is
692-
# excluded from the single-phase SPA profile above: on the SPH map those count PV
693-
# generation. Excluding a sensor that would have read zero costs nothing; including
694-
# one is the defect being fixed here.
707+
# ENERGY_SENSORS was excluded on the assumption that energy_today/energy_total count
708+
# PV generation on the SPH map, which an AC-coupled unit has none of. A full scan of
709+
# the #360 device refuted that:
710+
#
711+
# input 53/54 = 0/20 -> 2.0 kWh today
712+
# input 55/56 = 0/23139 -> 2313.9 kWh total
713+
#
714+
# and the SPA extended block agrees exactly at 2053-2056, which is a second address
715+
# reporting the same quantity. These count what the inverter puts out, and a battery
716+
# discharging through it produces output like anything else. Restored.
717+
#
718+
# Worth recording how close this came to shipping wrong: the reporter checked the
719+
# same registers and said there were no real values there, because 53 and 55 are the
720+
# HIGH words and both read zero. The data was in 54 and 56. The scan CSV settled in
721+
# seconds what two rounds of asking could not.
695722
"spa_tl3_4000_10000_v201": {
696723
"name": "SPA-TL3 (AC Storage) 4-10kW",
697724
"description": "Three-phase AC-coupled battery storage, no solar DC inputs (VPP V2.01)",
@@ -707,6 +734,7 @@
707734
POWER_FLOW_SENSORS |
708735
CONSUMPTION_SENSORS |
709736
ENERGY_BREAKDOWN_SENSORS |
737+
ENERGY_SENSORS |
710738
BATTERY_SENSORS |
711739
BMS_SENSORS |
712740
TEMPERATURE_SENSORS |
@@ -791,7 +819,8 @@
791819
"has_pv3": True,
792820
"has_battery": True,
793821
"max_power_kw": 15.0,
794-
"sensors": (HYBRID_3P_SENSORS | PV3_SENSORS | BACKUP_BOX_SENSORS) - NO_BATTERY_TEMP,
822+
"sensors": ((HYBRID_3P_SENSORS | PV3_SENSORS | BACKUP_BOX_SENSORS | DCDC_TEMP_SENSOR)
823+
- NO_BATTERY_TEMP),
795824
},
796825

797826
"mod_6000_15000tl3_xh_v201": {
@@ -803,7 +832,8 @@
803832
"has_pv3": True,
804833
"has_battery": True,
805834
"max_power_kw": 15.0,
806-
"sensors": (HYBRID_3P_SENSORS | PV3_SENSORS | BACKUP_BOX_SENSORS) - NO_BATTERY_TEMP,
835+
"sensors": ((HYBRID_3P_SENSORS | PV3_SENSORS | BACKUP_BOX_SENSORS | DCDC_TEMP_SENSOR)
836+
- NO_BATTERY_TEMP),
807837
},
808838

809839
# MID 11-30KTL3-XH / MID 8-15KTL3-XHL/JP — three-phase commercial hybrid
@@ -821,7 +851,8 @@
821851
"has_pv3": True,
822852
"has_battery": True,
823853
"max_power_kw": 30.0,
824-
"sensors": (HYBRID_3P_SENSORS | PV3_SENSORS | BACKUP_BOX_SENSORS) - NO_BATTERY_TEMP,
854+
"sensors": ((HYBRID_3P_SENSORS | PV3_SENSORS | BACKUP_BOX_SENSORS | DCDC_TEMP_SENSOR)
855+
- NO_BATTERY_TEMP),
825856
},
826857

827858
# ========================================================================

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

custom_components/growatt_modbus/profiles/sph_tl3.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,20 @@
9090
92: {'name': 'pv_energy_total_low', 'scale': 1, 'unit': '', 'pair': 91, 'combined_scale': 0.1, 'combined_unit': 'kWh', 'desc': 'Total PV energy lifetime LOW'},
9191

9292
# Temperatures
93+
#
94+
# 94 and 95 were missing while `ipm_temp` and `boost_temp` were both in this
95+
# profile's sensor set — so the entities existed and published 0.0 °C on every
96+
# SPH-TL3 and SPA-TL3 install. Not a hardware limit: a full scan of the #360
97+
# device read 94 = 200 (20.0 °C) and 95 = 333 (33.3 °C) alongside 93 = 378
98+
# (37.8 °C), all three plausible and independent.
99+
#
100+
# The VPP copies at 31130-31132 are mapped further down and read zero on that
101+
# device, so the fallback was never going to populate these either.
93102
93: {'name': 'inverter_temp', 'scale': 0.1, 'unit': '°C', 'signed': True},
103+
94: {'name': 'ipm_temp', 'scale': 0.1, 'unit': '°C', 'signed': True,
104+
'desc': 'IPM (power module) temperature (confirmed #360 scan)'},
105+
95: {'name': 'boost_temp', 'scale': 0.1, 'unit': '°C', 'signed': True,
106+
'desc': 'Boost converter temperature (confirmed #360 scan)'},
94107

95108
# Status
96109
105: {'name': 'fault_code', 'scale': 1, 'unit': ''},

tests/test_phantom_temperatures.py

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
"""Temperature sensors must have a register behind them (#360, #362).
2+
3+
A sensor in a profile's sensor set is created unconditionally. Its `condition` cannot
4+
save it when the attribute is a `GrowattData` field, because the field always exists with
5+
a default — so a temperature with no register publishes 0.0 °C forever rather than going
6+
unavailable. On a dashboard that is a battery or a power module sitting at freezing, which
7+
reads as a measurement rather than as missing data.
8+
9+
This is the #362 battery_temp bug. It recurred immediately: a full scan of the #360 device
10+
showed `ipm_temp` and `boost_temp` in the SPH-TL3 sensor set with nothing mapped to them,
11+
and `dcdc_temp` too — three phantom temperatures on hardware that has been confirmed since
12+
DTC 3601. Two were fixable by adding registers 94 and 95, which the device answers with
13+
plausible values; the third has no register anywhere in that map and had to be removed
14+
from the set.
15+
16+
Scoped to temperatures deliberately. The same check across every sensor produces mostly
17+
false positives, because sensors such as `status`, `last_update`, `grid_import_power` and
18+
`grid_export_power` are computed by the coordinator rather than read from an address. A
19+
noisy test gets ignored, and this class is worth catching precisely.
20+
"""
21+
from __future__ import annotations
22+
23+
import importlib
24+
25+
import pytest
26+
27+
_dp = importlib.import_module("growatt_under_test.device_profiles")
28+
_profiles = importlib.import_module("growatt_under_test.profiles")
29+
30+
INVERTER_PROFILES = _dp.INVERTER_PROFILES
31+
32+
TEMPERATURE_SENSORS = {
33+
"inverter_temp", "ipm_temp", "boost_temp", "dcdc_temp", "battery_temp",
34+
}
35+
36+
37+
def _register_names(profile_key: str) -> set[str]:
38+
"""Every name a profile's register map can produce, including combined pairs."""
39+
rmap = _profiles.get_profile(INVERTER_PROFILES[profile_key]["register_map"])
40+
assert rmap is not None, (
41+
f"{profile_key} names register map "
42+
f"{INVERTER_PROFILES[profile_key]['register_map']!r}, which does not resolve"
43+
)
44+
names: set[str] = set()
45+
for space in ("input_registers", "holding_registers"):
46+
for reg in rmap.get(space, {}).values():
47+
name = reg["name"]
48+
names.add(name)
49+
for suffix in ("_high", "_low"):
50+
if name.endswith(suffix):
51+
names.add(name[: -len(suffix)])
52+
return names
53+
54+
55+
# Profiles with pre-existing phantom temperatures, recorded rather than hidden.
56+
#
57+
# These predate the dcdc_temp regression and are a different problem: the sensor is
58+
# plausible for the hardware, but no register is mapped and there is no scan from one of
59+
# these models to say whether the device reports it. Fixing them blind means either
60+
# inventing an address or deleting a sensor that may work — both need a device.
61+
#
62+
# Listed explicitly so the count can only go down. A new profile with a phantom
63+
# temperature fails; these six are known debt, and removing a name from this list when a
64+
# scan settles it is the intended way to close them.
65+
KNOWN_PHANTOM_TEMPERATURES = {
66+
"mic_2500_5500mtl_s": {"boost_temp"},
67+
"mic_600_3300tl_x": {"boost_temp"},
68+
"mic_600_3300tl_x_v201": {"boost_temp"},
69+
"spe_8000_12000_es": {"battery_temp", "boost_temp", "ipm_temp"},
70+
"spf_3000_6000_es_plus": {"battery_temp", "boost_temp", "ipm_temp"},
71+
"tl3_s_3000_15000": {"boost_temp", "ipm_temp"},
72+
}
73+
74+
75+
@pytest.mark.parametrize("profile_key", sorted(INVERTER_PROFILES))
76+
def test_every_temperature_sensor_has_a_register(profile_key):
77+
declared = INVERTER_PROFILES[profile_key]["sensors"] & TEMPERATURE_SENSORS
78+
available = _register_names(profile_key)
79+
known = KNOWN_PHANTOM_TEMPERATURES.get(profile_key, set())
80+
phantom = sorted(declared - available - known)
81+
82+
# A name that stops being phantom must leave the list, or the list quietly becomes
83+
# a place where fixed things are still described as broken.
84+
stale = sorted(known - (declared - available))
85+
assert not stale, (
86+
f"{profile_key} lists {stale} as known phantom temperatures, but they now have "
87+
f"registers. Remove them from KNOWN_PHANTOM_TEMPERATURES."
88+
)
89+
90+
assert not phantom, (
91+
f"{profile_key} declares temperature sensors with no register to populate them: "
92+
f"{phantom}. They will be created and publish 0.0 °C. Either map the register, "
93+
f"or subtract the sensor from this profile's set — a hasattr() condition cannot "
94+
f"suppress it, because these are GrowattData fields with defaults."
95+
)

0 commit comments

Comments
 (0)