Skip to content

Commit 8a7aa74

Browse files
CIclaude
andcommitted
Merge origin/main into bot/debug-journal-2026-09-24
Resolve the tools/debug-journal.md conflict with the now-merged 2026-09-23 slice (#5199) by a word-level splice: the two slices edited disjoint spans of the same table rows. Where both appended a sentence to the end of the same cell (GivTCP REST, Component health) both are kept, #5199's text first. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
2 parents d052cf4 + 1daa164 commit 8a7aa74

13 files changed

Lines changed: 1229 additions & 97 deletions

‎.cspell/custom-dictionary-workspace.txt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,7 @@ pvbat
506506
pvenergytotal
507507
pvlib
508508
pvlimit
509+
PVONLY
509510
pvpower
510511
pvwatts
511512
pwindow
@@ -673,6 +674,7 @@ typecodes
673674
tzfile
674675
tzpath
675676
unasserted
677+
uncalibrated
676678
uncompilable
677679
unconfigured
678680
underivable

‎apps/predbat/coordinator.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -916,8 +916,9 @@ def inverter_record(
916916
Tuples become lists because validate_report() keeps only a list for a structural or
917917
vocabulary field: a tuple `serials` or `functions` - a module-level constant, say - would be
918918
silently dropped from the catalogue. A set or frozenset becomes a sorted list for the same
919-
reason, sorted because string hashing is randomised per process, so list(some_set) comes out
920-
in a different order after a restart and two dumps of the same hardware would diff for nothing.
919+
reason, sorted by string form so that a set mixing types cannot raise and withhold the whole
920+
report, and sorted because string hashing is randomised per process, so list(some_set) comes
921+
out in a different order after a restart and two dumps of the same hardware would diff for nothing.
921922
"""
922923
fields = {
923924
"inverter_type": inverter_type,
@@ -939,7 +940,7 @@ def inverter_record(
939940
record = {"device_id": device_id}
940941
for name, value in fields.items():
941942
if isinstance(value, (set, frozenset)):
942-
value = sorted(value)
943+
value = sorted(value, key=str)
943944
elif isinstance(value, (list, tuple)):
944945
value = list(value)
945946
elif isinstance(value, dict):

‎apps/predbat/fox.py‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import argparse
2727
import random
2828
from component_base import ComponentBase
29+
from coordinator import inverter_record
2930
from mock_base import MockBase
3031
from oauth_mixin import OAuthMixin
3132
from utils import dp2, load_apps_yaml
@@ -598,6 +599,18 @@ async def run(self, seconds, first):
598599
if first or settings_refresh or production_refresh or realtime_refresh:
599600
await self.publish_data()
600601

602+
# Unconditional, once per cycle and outside any one-shot gate, so a transient failure is
603+
# retried rather than lost - see ComponentBase.refresh_discovery(), which owns the
604+
# compare/guard loop, and never raises. Placed after the device poll and publish_data()
605+
# above so it describes what this cycle actually read.
606+
#
607+
# Deliberately BEFORE automatic_config() below, as GivTCP's run() does: automatic_config()
608+
# raises when no device qualifies as a battery inverter (a battery with no scheduler, say),
609+
# and that exception leaves run() every retry. Called after it, the report would never be
610+
# filed on exactly the installs whose debug dump most needs to say why. automatic_config()
611+
# reads nothing this call writes, so it still runs, and still raises, exactly as before.
612+
self.refresh_discovery()
613+
601614
# Automatic configuration on first run
602615
if first and self.automatic:
603616
await self.automatic_config()
@@ -2380,6 +2393,140 @@ async def automatic_config(self):
23802393
if len(batteries):
23812394
self.set_arg("battery_temperature_history", f"sensor.{self.prefix}_fox_{batteries[0]}_battemperature")
23822395

2396+
def build_discovery(self):
2397+
"""
2398+
Describe the discovered Fox devices for the discovery catalogue.
2399+
2400+
Reads only what automatic_config() already gathers - self.device_list, self.device_detail
2401+
and self.device_settings - so this adds no API calls and cannot change what Fox does.
2402+
Reporting is independent of self.automatic: the catalogue records what hardware is there,
2403+
not whether this component wired apps.yaml to it, which is what the report's own
2404+
"automatic" flag is for.
2405+
2406+
functions says what a device physically has ("solar", "battery"); inverter_type says
2407+
whether Predbat would drive it. inverter_type is "FoxCloud" - the INVERTER_DEF key
2408+
automatic_config() itself writes - on exactly the devices automatic_config() counts as
2409+
inverters: hasBattery AND function.scheduler AND a positive capacity. Any other device,
2410+
a PV-only one or a battery automatic_config() refuses (no scheduler, say), carries no
2411+
inverter_type but still shows its battery in functions, which is precisely what a dump
2412+
from an install whose auto-config fails needs to show. That predicate is duplicated here
2413+
rather than shared, because sharing it means changing automatic_config(), a control path;
2414+
automatic_config() is the source of truth, and a test runs it to pin this copy to it.
2415+
2416+
The inverter's rating goes through capacity_watts(), never capacity * 1000: Fox reports a
2417+
half-kW model's capacity truncated (a KH10.5 says 10), and capacity_watts() is what
2418+
restores the 500 W. For a battery device that is also the value of the _inverter_capacity
2419+
sensor publish_data() publishes; for a PV-only device publish_data() sets that sensor to
2420+
0, while the catalogue still reports the device's own rating.
2421+
2422+
stationName, stationID and moduleSN are deliberately not reported. stationName is
2423+
user-authored free text that can hold a street address (get_device_list()'s own sample
2424+
holds one), and the info container's guards - a length cap and no "@" - would let an
2425+
address straight into a debug dump users post publicly. Neither identifier describes the
2426+
hardware; a station ID, if one is ever wanted, belongs in account_ids, which is
2427+
pseudonymised.
2428+
2429+
No battery capacity is reported. A real batteryList mixes control units that carry no
2430+
capacity (bcu, ivu) with bmu entries carrying one in Wh, and publish_data() sums every
2431+
entry that carries one - a known live bug (GH#4919): an AIO ESS reports its one pack as
2432+
four bmu entries, each claiming the whole pack and all carrying the inverter's own
2433+
serial, so the sum is 4x the truth. Publishing it would put a knowingly-wrong battery_kwh
2434+
in every affected dump. Two ratings are reported instead, named for what the API returned
2435+
rather than as the spec's "modules", since the vendor figure is known to be wrong:
2436+
battery_capacity_entries, how many entries publish_data() sums, and
2437+
battery_capacity_serials, how many distinct batterySN those entries carry. A healthy
2438+
four-module stack reports four and four; four entries against one serial is the GH#4919
2439+
signature. Neither len(batteryList) nor the entry count alone can tell them apart - each
2440+
reads the same for the bug as for that healthy stack. Deliberately not gated on hasBattery:
2441+
a battery list on a device that says it has no battery is itself worth seeing.
2442+
2443+
Returns None when nothing has been discovered yet, which refresh_discovery() treats as
2444+
"nothing to report, ask again next cycle".
2445+
"""
2446+
if not self.device_list:
2447+
return None
2448+
2449+
inverters = []
2450+
for device in self.device_list:
2451+
serial = device.get("deviceSN")
2452+
if not serial:
2453+
continue
2454+
detail = self.device_detail.get(serial, {}) or {}
2455+
has_battery = bool(detail.get("hasBattery", False))
2456+
has_pv = bool(detail.get("hasPV", False))
2457+
has_scheduler = bool((detail.get("function") or {}).get("scheduler", False))
2458+
2459+
functions = []
2460+
if has_pv:
2461+
functions.append("solar")
2462+
if has_battery:
2463+
functions.append("battery")
2464+
2465+
# "schedule" is the spec's token for a device-side scheduler; export_limit has no spec
2466+
# equivalent. A third-party generator this inverter meters is topology, not something
2467+
# the inverter can do, so it is a flag rather than a capability.
2468+
capabilities = []
2469+
if has_scheduler:
2470+
capabilities.append("schedule")
2471+
settings = self.device_settings.get(serial, {}) or {}
2472+
# Matched case-insensitively, as automatic_config() does when it sets hasExportLimit
2473+
if any(str(name).lower() == "exportlimit" for name in settings):
2474+
capabilities.append("export_limit")
2475+
flags = []
2476+
if detail.get("thirdPartyGen", False):
2477+
flags.append("third_party_gen")
2478+
2479+
info = {}
2480+
device_type = detail.get("deviceType")
2481+
if device_type:
2482+
info["model"] = str(device_type)
2483+
product_type = detail.get("productType")
2484+
if product_type:
2485+
info["product_type"] = str(product_type)
2486+
# Fox reports a firmware version per board; info takes only strings, so they are
2487+
# flattened into one, board names in sorted order - as GE Cloud's
2488+
# _device_info_and_ratings() does with its firmware_version dict
2489+
boards = [(board, detail.get(board + "Version")) for board in ("manager", "master", "slave")]
2490+
firmware = " ".join("{} {}".format(board, version.strip()) for board, version in boards if isinstance(version, str) and version.strip())
2491+
if firmware:
2492+
info["firmware"] = firmware
2493+
2494+
# capacity_watts() multiplies the raw field, so only a positive number reaches it -
2495+
# the same test drives_it applies below. Anything else drops the rating, not the report.
2496+
capacity = detail.get("capacity", 0)
2497+
capacity_is_rating = isinstance(capacity, (int, float)) and capacity > 0
2498+
ratings = {}
2499+
if capacity_is_rating:
2500+
ratings["inverter_w"] = self.capacity_watts(detail)
2501+
battery_list = detail.get("batteryList") or []
2502+
summed = [entry for entry in battery_list if isinstance(entry, dict) and "capacity" in entry] if isinstance(battery_list, list) else []
2503+
if summed:
2504+
ratings["battery_capacity_entries"] = len(summed)
2505+
ratings["battery_capacity_serials"] = len({entry.get("batterySN") for entry in summed if entry.get("batterySN")})
2506+
2507+
# automatic_config() is the source of truth for which devices are inverters Predbat
2508+
# drives: it configures one only when hasBattery, function.scheduler and capacity > 0
2509+
# all hold. Duplicated here, not shared, so this observer does not touch that control
2510+
# path; test_fox_build_discovery_sets_inverter_type_only_where_automatic_config_would
2511+
# runs the real automatic_config() to keep the two in step.
2512+
drives_it = has_battery and has_scheduler and capacity_is_rating
2513+
2514+
inverters.append(
2515+
inverter_record(
2516+
"fox:{}".format(serial),
2517+
inverter_type="FoxCloud" if drives_it else None,
2518+
composition="direct",
2519+
functions=functions,
2520+
capabilities=capabilities,
2521+
flags=flags,
2522+
hardware_ids={"serial": serial},
2523+
info=info,
2524+
ratings=ratings,
2525+
)
2526+
)
2527+
2528+
return {"automatic": self.automatic, "inverters": inverters}
2529+
23832530

23842531
async def test_write_schedule(sn, api_key, token_hash, token_expires, supabase_url, supabase_key, user_id): # pragma: no cover
23852532
"""

‎apps/predbat/solis.py‎

Lines changed: 57 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,16 @@ async def encode_time_windows(self, inverter_sn, time_windows):
813813
first_slot = time_windows.get(1, time_windows.get(min(time_windows.keys())))
814814
field_length = first_slot.get("field_length", 18) # Default to variant 1
815815

816+
# Never encode a current above what the inverter can deliver at its rated power (issue #5187)
817+
rated_current = self.get_rated_current(inverter_sn)
818+
819+
def encoded_current(slot_data, key):
820+
"""Return a slot current as CID 103 holds it, in whole amps and no higher than the inverter's rating."""
821+
current = int(slot_data.get(key, 0))
822+
if rated_current is not None:
823+
current = min(current, int(rated_current))
824+
return str(current)
825+
816826
fields = []
817827

818828
if field_length == 18:
@@ -822,8 +832,8 @@ async def encode_time_windows(self, inverter_sn, time_windows):
822832
slot_data = time_windows.get(slot_index, {})
823833

824834
# Extract fields with defaults
825-
charge_current = str(int(slot_data.get("charge_current", 0)))
826-
discharge_current = str(int(slot_data.get("discharge_current", 0)))
835+
charge_current = encoded_current(slot_data, "charge_current")
836+
discharge_current = encoded_current(slot_data, "discharge_current")
827837
charge_start = slot_data.get("charge_start_time", "00:00")
828838
charge_end = slot_data.get("charge_end_time", "00:00")
829839
discharge_start = slot_data.get("discharge_start_time", "00:00")
@@ -839,8 +849,8 @@ async def encode_time_windows(self, inverter_sn, time_windows):
839849
slot_data = time_windows.get(slot_index, {})
840850

841851
# Extract fields with defaults
842-
charge_current = str(int(slot_data.get("charge_current", 0)))
843-
discharge_current = str(int(slot_data.get("discharge_current", 0)))
852+
charge_current = encoded_current(slot_data, "charge_current")
853+
discharge_current = encoded_current(slot_data, "discharge_current")
844854
charge_start = slot_data.get("charge_start_time", "00:00")
845855
charge_end = slot_data.get("charge_end_time", "00:00")
846856
discharge_start = slot_data.get("discharge_start_time", "00:00")
@@ -941,6 +951,11 @@ async def resolve_discharge_soc_floor(self, inverter_sn, target_soc):
941951
over-discharge SOC is never written here, leaving the battery protection floor
942952
where the user set it.
943953
954+
A recovery SOC reading at or below the over-discharge SOC, or above 100, cannot be
955+
right, yet most inverters report one - 0, 1, the over-discharge SOC itself or 65521
956+
(issue #5187). Such a reading is replaced by over-discharge + 1, where the inverter
957+
holds the recovery SOC, and is never written back.
958+
944959
Args:
945960
inverter_sn: Inverter serial number
946961
target_soc: Cut-off SOC Predbat wants for the discharge slot, as a percentage
@@ -952,11 +967,17 @@ async def resolve_discharge_soc_floor(self, inverter_sn, target_soc):
952967
# Cache only - the infrequent poll runs before the first control write, so a missing
953968
# value means we cannot tell where the floor is and the target is left alone
954969
recovery_soc = parse_cid_int(values.get(SOLIS_CID_BATTERY_RECOVERY_SOC))
970+
# The inverter will not accept a recovery SOC at or below the over-discharge SOC
971+
over_discharge_soc = parse_cid_int(values.get(SOLIS_CID_BATTERY_OVER_DISCHARGE_SOC))
972+
973+
recovery_reading = recovery_soc
974+
if recovery_soc is not None and (recovery_soc > 100 or (over_discharge_soc is not None and recovery_soc <= over_discharge_soc)):
975+
recovery_soc = (over_discharge_soc + 1) if over_discharge_soc is not None else None
955976
if recovery_soc is None or target_soc >= recovery_soc:
956977
return target_soc
978+
if recovery_soc != recovery_reading:
979+
self.log(f"Solis API: Recovery SOC on {inverter_sn} reads {recovery_reading}%, which cannot be right with over-discharge SOC at {over_discharge_soc}%, so taking it as {recovery_soc}%")
957980

958-
# The inverter will not accept a recovery SOC at or below the over-discharge SOC
959-
over_discharge_soc = parse_cid_int(values.get(SOLIS_CID_BATTERY_OVER_DISCHARGE_SOC))
960981
hard_floor = (over_discharge_soc + 1) if over_discharge_soc is not None else recovery_soc
961982

962983
wanted_recovery = max(target_soc, hard_floor)
@@ -973,6 +994,28 @@ async def resolve_discharge_soc_floor(self, inverter_sn, target_soc):
973994
return recovery_soc
974995
return target_soc
975996

997+
def get_rated_current(self, inverter_sn):
998+
"""Return the battery current the inverter can deliver at its rated power, in whole amps, or None if its size is unknown.
999+
1000+
The rating is inverterDetail power - the sensor published as inverter_size and bound to
1001+
inverter_limit - converted at get_nominal_voltage(), which holds still on an LV pack.
1002+
Floored to whole amps so the capped value is never above the rating, and so an HV pack,
1003+
converted at its live voltage, does not produce a new value to write every cycle.
1004+
"""
1005+
detail = self.inverter_details.get(inverter_sn, {})
1006+
try:
1007+
power = float(detail.get("power"))
1008+
except (ValueError, TypeError):
1009+
return None
1010+
unit = str(detail.get("powerStr") or "kW").strip().lower()
1011+
if unit == "kw":
1012+
power *= 1000.0
1013+
elif unit != "w":
1014+
return None
1015+
if power <= 0:
1016+
return None
1017+
return float(int(round(power / self.get_nominal_voltage(inverter_sn), 6)))
1018+
9761019
async def write_time_windows_if_changed(self, inverter_sn):
9771020
"""Write charge/discharge time windows, SOC, and current to inverter, only if values changed from cache.
9781021
Automatically handles V1 vs V2 modes and only writes registers that have changed.
@@ -1009,6 +1052,14 @@ async def write_time_windows_if_changed(self, inverter_sn):
10091052
current_cid = SOLIS_CID_DISCHARGE_CURRENT[slot - 1]
10101053
max_discharge_current_amps = min(self.cached_infos.get(inverter_sn, {}).get(current_cid, {}).get('sysCommand', {}).get('max', max_discharge_current_amps), max_discharge_current_amps)
10111054

1055+
# Then cap both at what the inverter can deliver at its rated power. CID 7224/7226 are battery limits: a
1056+
# 3.6kW inverter reading 100A there refused every 100A write and left its discharge slot at 0A (issue #5187)
1057+
rated_current = self.get_rated_current(inverter_sn)
1058+
if rated_current is not None and rated_current < max(max_charge_current_amps, max_discharge_current_amps):
1059+
self.log(f"Solis API: Capping slot currents on {inverter_sn} at {rated_current}A, the most the inverter can deliver at its rated power")
1060+
max_charge_current_amps = min(max_charge_current_amps, rated_current)
1061+
max_discharge_current_amps = min(max_discharge_current_amps, rated_current)
1062+
10121063
# Prep: extract active currents from slot 1 and zero out times for disabled slots
10131064
# so that the two-pass write below has clean data to compare against.
10141065
for slot in slots_to_check:

0 commit comments

Comments
 (0)