From 86f3b4516ad8d6faee4b0ac7f4e14d1440ec63ef Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Apr 2026 18:01:45 +0000 Subject: [PATCH 1/4] Add repro test for mixed DC+AC battery charging bug (#338) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a DC-only battery (B2500) is paired with an AC battery (Venus) in the same CT002 pool, LoadBalancer splits any grid surplus evenly across every reporter. The B2500 is charge-blind, so it silently ignores its half, and the Venus only ever sees half the real surplus — which can sit below its inverter's start-up threshold, leaving the Venus stuck in standby and the full surplus feeding to the grid. Adds a direct LoadBalancer-driven test that models a B2500-like charge-capped battery and a Venus-like battery with a start-up threshold, and asserts the observed symptom under a 600 W surplus. Includes solo-Venus and discharging controls to confirm the failure is specific to the DC+AC mix under surplus. --- tests/test_balancer_mixed_battery_charging.py | 285 ++++++++++++++++++ 1 file changed, 285 insertions(+) create mode 100644 tests/test_balancer_mixed_battery_charging.py diff --git a/tests/test_balancer_mixed_battery_charging.py b/tests/test_balancer_mixed_battery_charging.py new file mode 100644 index 00000000..7321fde0 --- /dev/null +++ b/tests/test_balancer_mixed_battery_charging.py @@ -0,0 +1,285 @@ +"""Reproduction for issue #338: mixed DC + AC batteries leave the AC +battery stuck in standby under solar surplus. + +Scenario from the report (users c00LhaNd86 and Matze6989): +a Marstek Venus (can charge **and** discharge via AC) runs next to a +Marstek B2500 (a DC battery — discharges via AC, but cannot charge via +AC at all). Both report to the same AstraMeter CT002 emulator fed by +a Shelly Pro 3EM. With several hundred watts of solar surplus the +Venus stays in standby indefinitely; pointing the Marstek app directly +at the Shelly (no AstraMeter in the loop) makes the Venus charge at +~1 kW immediately. Discharging, where the B2500 can participate, works +fine. + +Root cause: :meth:`LoadBalancer.compute_target` splits the grid reading +evenly across every reporting storage unit +(``fair_share = grid_total / N`` plus a ``_balance_correction`` that +pushes each consumer toward the average of all reported powers). +Neither mechanism knows the B2500 is charge-blind, so under surplus +each battery is asked to absorb half of the real feed-in. In the user's +setup that half falls below the Venus's inverter start-up threshold +(empirically ~300-500 W on a Venus E), so the Venus's controller keeps +deciding the command is too small to wake up — and because it never +starts, the grid stays at full surplus and the split never grows. + +This test drives :class:`LoadBalancer.compute_target` directly, mirroring +the harness used by :mod:`tests.test_balancer_empty_battery_lockup`, +and asserts the observable outcome: under a 600 W surplus the Venus +never leaves standby and the grid keeps feeding back the full 600 W. +""" + +from __future__ import annotations + +import time + +from astrameter.ct002.balancer import ( + BalancerConfig, + ConsumerMode, + LoadBalancer, +) + + +class _FakeClock: + def __init__(self) -> None: + self._t = time.time() + + def __call__(self) -> float: + return self._t + + def advance(self, dt: float) -> None: + self._t += dt + + +class DCOnlyBattery: + """B2500-style battery: discharges via AC, cannot charge via AC. + + Any negative (charge) command is clamped to 0. Positive commands + ramp up like a normal inverter within ``max_discharge``. + """ + + def __init__(self, mac: str, max_discharge: int = 800, ramp: float = 300.0) -> None: + self.mac = mac + self.max_discharge = max_discharge + self.ramp = ramp + self.power = 0.0 + + def step(self, target_delta: float, reported_power: float) -> None: + desired = reported_power + target_delta + desired = max(0, min(self.max_discharge, desired)) + delta = desired - self.power + if delta > self.ramp: + delta = self.ramp + elif delta < -self.ramp: + delta = -self.ramp + self.power += delta + + +class ACBatteryWithStartupThreshold: + """Venus-style battery with a start-up threshold below which it stays idle. + + Real Marstek Venus inverters will not transition out of standby + until the commanded change exceeds a few hundred watts — 400 W is + a conservative mid-point of what users report. Once activated the + battery ramps normally; if the commanded magnitude drops back below + ``startup_min`` for long enough the battery returns to standby. + The commanded magnitude is derived from the CT-style ``current + + delta`` protocol, matching :class:`astrameter.simulator.battery.BatterySimulator`. + """ + + def __init__( + self, + mac: str, + *, + max_charge: int = 2500, + max_discharge: int = 800, + ramp: float = 300.0, + startup_min: float = 400.0, + ) -> None: + self.mac = mac + self.max_charge = max_charge + self.max_discharge = max_discharge + self.ramp = ramp + self.startup_min = startup_min + self.power = 0.0 + self._active = False + + def step(self, target_delta: float, reported_power: float) -> None: + desired = reported_power + target_delta + desired = max(-self.max_charge, min(self.max_discharge, desired)) + if not self._active: + if abs(desired) < self.startup_min: + # Inverter stays in standby; no ramp, no power draw. + return + self._active = True + delta = desired - self.power + if delta > self.ramp: + delta = self.ramp + elif delta < -self.ramp: + delta = -self.ramp + self.power += delta + # If the Venus is commanded back near zero for a sustained period + # it drops back to standby — match the real inverter. + if self._active and abs(self.power) < 20 and abs(desired) < self.startup_min: + self._active = False + self.power = 0.0 + + +def _make_balancer(clock: _FakeClock) -> LoadBalancer: + """Balancer with CT002 defaults (matching the out-of-the-box config).""" + return LoadBalancer( + config=BalancerConfig( + fair_distribution=True, + balance_gain=0.2, + balance_deadband=15, + error_boost_threshold=150, + error_boost_max=0.5, + error_reduce_threshold=20, + max_correction_per_step=80, + # ``min_efficient_power=0`` disables efficiency deprioritization, + # matching the out-of-the-box config the reporters are running. + min_efficient_power=0, + probe_min_power=80, + efficiency_rotation_interval=900, + efficiency_fade_alpha=0.15, + efficiency_saturation_threshold=0.4, + ), + saturation_alpha=0.15, + saturation_min_target=20, + saturation_decay_factor=0.995, + saturation_grace_seconds=90.0, + saturation_stall_timeout_seconds=60.0, + saturation_enabled=True, + clock=clock, + ) + + +def _run_scenario(batteries, surplus_watts: float, ticks: int): + """Drive the balancer for *ticks* seconds at 1 Hz under a fixed surplus. + + Returns ``(grid_trace, per_mac_power_trace)``. + """ + clock = _FakeClock() + lb = _make_balancer(clock) + grid_trace: list[float] = [] + power_trace: dict[str, list[float]] = {b.mac: [] for b in batteries} + + for tick in range(ticks): + reports = {b.mac: {"phase": "A", "power": round(b.power)} for b in batteries} + # Grid = (load - solar) - battery_sum. Here we model a clean + # surplus-only case: zero house load, ``surplus_watts`` of solar, + # so ``grid = -surplus_watts - sum(battery.power)``. + grid_total = -surplus_watts - sum(b.power for b in batteries) + grid_trace.append(grid_total) + + deltas: dict[str, float] = {} + for b in batteries: + phase_targets = lb.compute_target( + consumer_id=b.mac, + consumer_mode=ConsumerMode("auto"), + all_reports=reports, + grid_total=grid_total, + inactive=frozenset(), + manual=frozenset(), + sample_id=(tick,), + ) + deltas[b.mac] = phase_targets[0] + + for b in batteries: + b.step(deltas[b.mac], reports[b.mac]["power"]) + power_trace[b.mac].append(b.power) + + clock.advance(1.0) + + return grid_trace, power_trace + + +def test_venus_never_wakes_up_with_b2500_present_under_surplus() -> None: + """600 W surplus + DC-only B2500 keeps the Venus stuck in standby. + + With a 600 W surplus the balancer commands each battery to absorb + 300 W. The B2500 can't charge, and the Venus's inverter never sees + a command above its start-up threshold (400 W here), so it never + activates. Because the Venus stays at 0 W, the B2500 at 0 W, and + the balancer can't see that the split is wrong, the grid keeps + feeding back the full 600 W forever. This is the exact observation + from issue #338. + """ + b2500 = DCOnlyBattery("b2500_01") + venus = ACBatteryWithStartupThreshold("venus_01", startup_min=400.0) + + grid, power = _run_scenario([b2500, venus], surplus_watts=600.0, ticks=200) + + venus_tail = power["venus_01"][-30:] + b2500_tail = power["b2500_01"][-30:] + grid_tail = grid[-30:] + + # B2500 is physically incapable of charging — always 0 W. + assert max(abs(p) for p in b2500_tail) < 1.0, ( + f"B2500 should stay at 0 W (DC-only), tail was {b2500_tail}" + ) + + # The Venus never leaves standby because the balancer-split command + # (-300 W) stays below its start-up threshold (400 W). + assert max(abs(p) for p in venus_tail) < 1.0, ( + f"Bug reproduced: Venus should be stuck at 0 W (standby), " + f"but observed tail {venus_tail}. If this assertion fails the " + f"bug may be fixed — verify against issue #338 before adjusting." + ) + + # Therefore the grid keeps feeding the full 600 W surplus back. + avg_grid = sum(grid_tail) / len(grid_tail) + assert avg_grid < -550, ( + f"Grid should still show ~-600 W feed-in (nothing is absorbing), " + f"average was {avg_grid:.0f} W" + ) + + +def test_venus_alone_absorbs_the_same_surplus_fine() -> None: + """Control: the same Venus without the B2500 drains the surplus. + + Confirms the failure is the DC + AC mix, not a general balancer or + threshold problem: with only the Venus in the pool, the full 600 W + surplus lands on the Venus, clears its start-up threshold, and it + absorbs the surplus to near-zero grid. + """ + venus = ACBatteryWithStartupThreshold("venus_solo", startup_min=400.0) + + grid, power = _run_scenario([venus], surplus_watts=600.0, ticks=200) + + venus_tail = power["venus_solo"][-30:] + grid_tail = grid[-30:] + + # Venus woke up and absorbed the surplus. + assert min(venus_tail) < -500, ( + f"Venus (solo) should absorb ~-600 W, tail was {venus_tail}" + ) + avg_grid = sum(grid_tail) / len(grid_tail) + assert abs(avg_grid) < 30, f"Grid should converge near 0 W, got {avg_grid:.0f} W" + + +def test_discharging_still_works_with_mixed_batteries() -> None: + """Control: the user notes discharging works; verify it here. + + With a positive grid import (house consuming more than solar), both + batteries can contribute — the B2500 by discharging normally, the + Venus likewise — so splitting the target evenly is fine. + """ + b2500 = DCOnlyBattery("b2500_dc_dis") + venus = ACBatteryWithStartupThreshold("venus_ac_dis", startup_min=400.0) + + # 1000 W of house consumption with no solar → grid imports 1000 W; + # ``_run_scenario`` uses ``-surplus_watts`` so we pass a negative + # "surplus" to simulate import. + grid, power = _run_scenario([b2500, venus], surplus_watts=-1000.0, ticks=200) + + avg_grid = sum(grid[-30:]) / 30 + # Both batteries discharge ~500 W each — grid drained to ~0. + assert abs(avg_grid) < 50, ( + f"Discharge across both batteries should drain grid, got {avg_grid:.0f} W" + ) + assert sum(power["b2500_dc_dis"][-30:]) / 30 > 400, ( + "B2500 should be actively discharging" + ) + assert sum(power["venus_ac_dis"][-30:]) / 30 > 400, ( + "Venus should also be discharging" + ) From 726e202d5aa62c6b1830ab016e5ad9ac03ab5350 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 23 Apr 2026 19:56:54 +0000 Subject: [PATCH 2/4] Exclude DC-only batteries from charge distribution (#338) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Real Marstek batteries advertise their model in the first field of the CT002 UDP request. The only AC-coupled family is the Venus lineup (``HMG`` and ``VNS`` prefixes); every other family (B2500 variants, Jupiter, future DC models) can discharge via AC but cannot charge. Previously the balancer split every surplus evenly across all reporters, asking DC-only batteries to charge — which they silently ignored, leaving the Venus with only half the real surplus. If that half sat below the Venus's inverter start-up threshold, the Venus stayed in standby and the full surplus kept feeding back to the grid. ``_compute_auto_target`` now treats any consumer whose ``device_type`` does not match a recognised AC prefix as DC-only and excludes it from charge distribution under ``grid_total < 0``. Positive grid_total (discharge) behaviour is unchanged; DC batteries continue to share the import load normally. Empty / unknown device types default to DC (fail-closed). If no recognised AC-chargeable battery is reporting under surplus, the balancer logs a single info-level notice and holds everyone at 0 W. Updates ``test_negative_target_concentrates`` to provide a Venus device_type for the two batteries under test — the efficiency pipeline it exercises is unrelated to the new guard. Test coverage: * b2500_excluded_from_charge_share_lets_venus_wake — canonical fix * all_known_venus_prefixes_are_charge_capable — HMG/VNS variants * non_venus_prefixes_are_treated_as_dc — HMA/HMB/HMJ/HMK/Jupiter * dc_discharge_still_shared_with_ac_sibling — discharge untouched * all_dc_under_surplus_holds_zero_and_logs — degenerate case * unknown_device_type_deadlock_persists — fail-closed default --- src/astrameter/ct002/balancer.py | 62 ++++ src/astrameter/ct002/ct002.py | 2 +- tests/test_balancer_mixed_battery_charging.py | 312 ++++++++++++------ tests/test_ct002_active_control.py | 7 +- 4 files changed, 286 insertions(+), 97 deletions(-) diff --git a/src/astrameter/ct002/balancer.py b/src/astrameter/ct002/balancer.py index 47b37967..8a3da23c 100644 --- a/src/astrameter/ct002/balancer.py +++ b/src/astrameter/ct002/balancer.py @@ -35,6 +35,24 @@ # rather than dosing the EMA with a huge rise or decay step. SATURATION_LONG_GAP_SECONDS = 30.0 +# Device-type prefixes of the only Marstek battery families that can charge +# via AC (the Venus lineup). ``HMG`` covers HMG-*; ``VNS`` covers VNSE3, +# VNSA, VNSD, and any other Venus-family variant. Every other reporting +# battery is assumed DC-coupled (B2500 family, Jupiter, etc.) and is +# excluded from charge distribution under a grid surplus. See issue #338. +AC_CHARGEABLE_DEVICE_PREFIXES: tuple[str, ...] = ("HMG", "VNS") + + +def _is_ac_chargeable(device_type: str) -> bool: + """True iff *device_type* identifies an AC-chargeable Marstek battery. + + Empty / unknown device types are treated as DC-only — an unknown + battery cannot be assumed to accept charge commands. + """ + if not device_type: + return False + return device_type.upper().startswith(AC_CHARGEABLE_DEVICE_PREFIXES) + # --------------------------------------------------------------------------- # Configuration @@ -310,6 +328,9 @@ def __init__( self._probe_success_threshold = max(1.0, float(saturation_min_target)) self._post_probe_fade_until = 0.0 self._post_probe_fade_ids: set[str] = set() + # Latch so the "surplus with no AC-chargeable battery" notice is + # logged once per transition into that state, not every tick. + self._all_dc_surplus_warned: bool = False def _get_consumer(self, consumer_id: str) -> BalancerConsumerState: state = self._consumers.get(consumer_id) @@ -811,6 +832,23 @@ def _compute_auto_target( num_consumers = max(1, len(reports)) eff_part = {cid: max(0.01, 1.0 - saturation.get(cid, 0.0)) for cid in reports} + # Exclude DC-only batteries (B2500 family, Jupiter, anything not in + # AC_CHARGEABLE_DEVICE_PREFIXES) from charge distribution under + # surplus. They'll happily discharge on positive grid_total, but + # asking them to charge wastes the share that should have gone to + # the AC sibling (Venus). See issue #338. + charge_blind = ( + { + cid + for cid, r in reports.items() + if not _is_ac_chargeable(r.get("device_type", "")) + } + if grid_total < 0 + else set() + ) + for cid in charge_blind: + eff_part[cid] = 0.0 + efficiency_adjustments = self._compute_efficiency_deprioritized( reports, sample_id, grid_total ) @@ -825,6 +863,30 @@ def _compute_auto_target( if probe_target is not None: return probe_target + # Degenerate case: every reporter is DC-only but we're under + # surplus. Nothing can absorb; log once so the user can see why + # the grid is still feeding back, then fall through to the + # per-consumer DC hold below. + all_dc_under_surplus = ( + grid_total < 0 and charge_blind and not (set(reports) - charge_blind) + ) + if all_dc_under_surplus and not self._all_dc_surplus_warned: + logger.info( + "CT002: %.0f W surplus but no AC-chargeable battery " + "reporting — holding all at 0 W. Reporting device_types: %s", + -grid_total, + sorted({reports[cid].get("device_type", "") or "?" for cid in reports}), + ) + self._all_dc_surplus_warned = True + elif not all_dc_under_surplus: + self._all_dc_surplus_warned = False + + # A DC-only consumer under surplus must be told explicitly to hold + # at 0 — don't fall through to the fair-share math where a residual + # correction could leak a nonzero target. + if consumer_id and consumer_id in charge_blind: + return self._steer_to_zero(consumer_id, reports) + # --- Fading path --- if any_fading and consumer_id: state = self._get_consumer(consumer_id) diff --git a/src/astrameter/ct002/ct002.py b/src/astrameter/ct002/ct002.py index 528cd5c0..9bb8cd0b 100644 --- a/src/astrameter/ct002/ct002.py +++ b/src/astrameter/ct002/ct002.py @@ -331,7 +331,7 @@ def _compute_smooth_target(self, values, consumer_id=None): mode = self._consumer_mode(consumer_id) reports = { - cid: {"phase": c.phase, "power": c.power} + cid: {"phase": c.phase, "power": c.power, "device_type": c.device_type} for cid, c in self._consumers.items() if c.timestamp > 0 } diff --git a/tests/test_balancer_mixed_battery_charging.py b/tests/test_balancer_mixed_battery_charging.py index 7321fde0..d6987f3e 100644 --- a/tests/test_balancer_mixed_battery_charging.py +++ b/tests/test_balancer_mixed_battery_charging.py @@ -1,37 +1,45 @@ -"""Reproduction for issue #338: mixed DC + AC batteries leave the AC -battery stuck in standby under solar surplus. - -Scenario from the report (users c00LhaNd86 and Matze6989): -a Marstek Venus (can charge **and** discharge via AC) runs next to a -Marstek B2500 (a DC battery — discharges via AC, but cannot charge via -AC at all). Both report to the same AstraMeter CT002 emulator fed by -a Shelly Pro 3EM. With several hundred watts of solar surplus the -Venus stays in standby indefinitely; pointing the Marstek app directly -at the Shelly (no AstraMeter in the loop) makes the Venus charge at -~1 kW immediately. Discharging, where the B2500 can participate, works -fine. - -Root cause: :meth:`LoadBalancer.compute_target` splits the grid reading -evenly across every reporting storage unit -(``fair_share = grid_total / N`` plus a ``_balance_correction`` that -pushes each consumer toward the average of all reported powers). -Neither mechanism knows the B2500 is charge-blind, so under surplus -each battery is asked to absorb half of the real feed-in. In the user's -setup that half falls below the Venus's inverter start-up threshold -(empirically ~300-500 W on a Venus E), so the Venus's controller keeps -deciding the command is too small to wake up — and because it never -starts, the grid stays at full surplus and the split never grows. - -This test drives :class:`LoadBalancer.compute_target` directly, mirroring -the harness used by :mod:`tests.test_balancer_empty_battery_lockup`, -and asserts the observable outcome: under a 600 W surplus the Venus -never leaves standby and the grid keeps feeding back the full 600 W. +"""Tests for issue #338: mixed DC + AC batteries under solar surplus. + +Scenario from the report (users c00LhaNd86 and Matze6989): a Marstek +Venus (can charge **and** discharge via AC) runs next to a Marstek +B2500 (a DC battery — discharges via AC, but cannot charge via AC at +all). Both report to the same AstraMeter CT002 emulator fed by a +Shelly Pro 3EM. Under solar surplus the Venus stayed in standby +indefinitely; pointing the Marstek app directly at the Shelly made the +Venus charge immediately. Discharging, where the B2500 can +participate, worked fine. + +Root cause: ``LoadBalancer._compute_auto_target`` split the grid +reading evenly across every reporting storage unit (fair-share plus a +``_balance_correction`` that pushed each consumer toward the average +of reported powers). Neither mechanism knew the B2500 was +charge-blind, so under surplus each battery was told to absorb half +of the real feed-in — often below the Venus's inverter start-up +threshold (~300-500 W), so the Venus never woke up. + +Fix: real Marstek batteries advertise their model in the CT002 request +(``Consumer.device_type``). The only AC-coupled family is the Venus +(prefixes ``HMG`` and ``VNS``). ``_compute_auto_target`` now excludes +every other reporter from charge distribution under ``grid_total < 0`` +and steers them to 0 W; the Venus receives the full surplus. +Positive grid (discharge) behaviour is unchanged. + +The tests here cover: + + * the legacy deadlock when ``device_type`` is missing (proves the + default-to-DC safety policy is in effect), + * the fix path with recognised Venus / B2500 prefixes, + * discharge unaffected, + * the degenerate all-DC-under-surplus case. """ from __future__ import annotations +import logging import time +import pytest + from astrameter.ct002.balancer import ( BalancerConfig, ConsumerMode, @@ -57,10 +65,18 @@ class DCOnlyBattery: ramp up like a normal inverter within ``max_discharge``. """ - def __init__(self, mac: str, max_discharge: int = 800, ramp: float = 300.0) -> None: + def __init__( + self, + mac: str, + *, + max_discharge: int = 800, + ramp: float = 300.0, + device_type: str = "HMJ-1", + ) -> None: self.mac = mac self.max_discharge = max_discharge self.ramp = ramp + self.device_type = device_type self.power = 0.0 def step(self, target_delta: float, reported_power: float) -> None: @@ -82,8 +98,9 @@ class ACBatteryWithStartupThreshold: a conservative mid-point of what users report. Once activated the battery ramps normally; if the commanded magnitude drops back below ``startup_min`` for long enough the battery returns to standby. - The commanded magnitude is derived from the CT-style ``current + - delta`` protocol, matching :class:`astrameter.simulator.battery.BatterySimulator`. + The commanded magnitude is derived from the CT-style + ``current + delta`` protocol, matching + :class:`astrameter.simulator.battery.BatterySimulator`. """ def __init__( @@ -94,12 +111,14 @@ def __init__( max_discharge: int = 800, ramp: float = 300.0, startup_min: float = 400.0, + device_type: str = "HMG-50", ) -> None: self.mac = mac self.max_charge = max_charge self.max_discharge = max_discharge self.ramp = ramp self.startup_min = startup_min + self.device_type = device_type self.power = 0.0 self._active = False @@ -108,7 +127,6 @@ def step(self, target_delta: float, reported_power: float) -> None: desired = max(-self.max_charge, min(self.max_discharge, desired)) if not self._active: if abs(desired) < self.startup_min: - # Inverter stays in standby; no ramp, no power draw. return self._active = True delta = desired - self.power @@ -117,8 +135,6 @@ def step(self, target_delta: float, reported_power: float) -> None: elif delta < -self.ramp: delta = -self.ramp self.power += delta - # If the Venus is commanded back near zero for a sustained period - # it drops back to standby — match the real inverter. if self._active and abs(self.power) < 20 and abs(desired) < self.startup_min: self._active = False self.power = 0.0 @@ -156,7 +172,9 @@ def _make_balancer(clock: _FakeClock) -> LoadBalancer: def _run_scenario(batteries, surplus_watts: float, ticks: int): """Drive the balancer for *ticks* seconds at 1 Hz under a fixed surplus. - Returns ``(grid_trace, per_mac_power_trace)``. + Each battery contributes its ``device_type`` to the report dict, so + the fix's AC-allow-list can see it. Returns + ``(grid_trace, per_mac_power_trace)``. """ clock = _FakeClock() lb = _make_balancer(clock) @@ -164,7 +182,14 @@ def _run_scenario(batteries, surplus_watts: float, ticks: int): power_trace: dict[str, list[float]] = {b.mac: [] for b in batteries} for tick in range(ticks): - reports = {b.mac: {"phase": "A", "power": round(b.power)} for b in batteries} + reports = { + b.mac: { + "phase": "A", + "power": round(b.power), + "device_type": b.device_type, + } + for b in batteries + } # Grid = (load - solar) - battery_sum. Here we model a clean # surplus-only case: zero house load, ``surplus_watts`` of solar, # so ``grid = -surplus_watts - sum(battery.power)``. @@ -193,19 +218,21 @@ def _run_scenario(batteries, surplus_watts: float, ticks: int): return grid_trace, power_trace -def test_venus_never_wakes_up_with_b2500_present_under_surplus() -> None: - """600 W surplus + DC-only B2500 keeps the Venus stuck in standby. +# --------------------------------------------------------------------------- +# Fix path — recognised Marstek device_types +# --------------------------------------------------------------------------- + - With a 600 W surplus the balancer commands each battery to absorb - 300 W. The B2500 can't charge, and the Venus's inverter never sees - a command above its start-up threshold (400 W here), so it never - activates. Because the Venus stays at 0 W, the B2500 at 0 W, and - the balancer can't see that the split is wrong, the grid keeps - feeding back the full 600 W forever. This is the exact observation - from issue #338. +def test_b2500_excluded_from_charge_share_lets_venus_wake() -> None: + """The canonical fix scenario: HMJ-1 B2500 + HMG-50 Venus under 600 W surplus. + + With the device-type prefix check in place the B2500 is excluded + from charge distribution, so the Venus receives the full -600 W + command on tick 0, clears its start-up threshold, and absorbs the + surplus to near-zero grid. """ - b2500 = DCOnlyBattery("b2500_01") - venus = ACBatteryWithStartupThreshold("venus_01", startup_min=400.0) + b2500 = DCOnlyBattery("b2500_01", device_type="HMJ-1") + venus = ACBatteryWithStartupThreshold("venus_01", device_type="HMG-50") grid, power = _run_scenario([b2500, venus], surplus_watts=600.0, ticks=200) @@ -213,73 +240,170 @@ def test_venus_never_wakes_up_with_b2500_present_under_surplus() -> None: b2500_tail = power["b2500_01"][-30:] grid_tail = grid[-30:] - # B2500 is physically incapable of charging — always 0 W. assert max(abs(p) for p in b2500_tail) < 1.0, ( - f"B2500 should stay at 0 W (DC-only), tail was {b2500_tail}" + f"B2500 should remain at 0 W throughout (DC-only). Tail: {b2500_tail}" ) - - # The Venus never leaves standby because the balancer-split command - # (-300 W) stays below its start-up threshold (400 W). - assert max(abs(p) for p in venus_tail) < 1.0, ( - f"Bug reproduced: Venus should be stuck at 0 W (standby), " - f"but observed tail {venus_tail}. If this assertion fails the " - f"bug may be fixed — verify against issue #338 before adjusting." + assert min(venus_tail) < -500, ( + f"Venus should absorb most of the 600 W surplus. Tail: {venus_tail}" ) - - # Therefore the grid keeps feeding the full 600 W surplus back. avg_grid = sum(grid_tail) / len(grid_tail) - assert avg_grid < -550, ( - f"Grid should still show ~-600 W feed-in (nothing is absorbing), " - f"average was {avg_grid:.0f} W" - ) + assert abs(avg_grid) < 50, f"Grid should drain near 0 W, got {avg_grid:.0f} W" + + +@pytest.mark.parametrize( + "venus_device_type", + [ + "HMG-50", + "hmg-50", + "HMG", + "VNSE3-X", + "vnse3-x", + "VNSA-1", + "VNSD-2", + "VNS", + ], +) +def test_all_known_venus_prefixes_are_charge_capable(venus_device_type: str) -> None: + """Every recognised Venus-family prefix must absorb the surplus. + ``HMG`` covers the older HMG-* naming; ``VNS`` covers VNSE3, VNSA, + VNSD, and any bare-prefix variant. Lowercase and suffixed forms + must all match — the lookup is case-insensitive and prefix-based. + """ + b2500 = DCOnlyBattery("b2500", device_type="HMJ-1") + venus = ACBatteryWithStartupThreshold("venus", device_type=venus_device_type) -def test_venus_alone_absorbs_the_same_surplus_fine() -> None: - """Control: the same Venus without the B2500 drains the surplus. + _, power = _run_scenario([b2500, venus], surplus_watts=600.0, ticks=200) - Confirms the failure is the DC + AC mix, not a general balancer or - threshold problem: with only the Venus in the pool, the full 600 W - surplus lands on the Venus, clears its start-up threshold, and it - absorbs the surplus to near-zero grid. - """ - venus = ACBatteryWithStartupThreshold("venus_solo", startup_min=400.0) + assert min(power["venus"][-30:]) < -500, ( + f"Venus with device_type={venus_device_type!r} should charge; " + f"tail was {power['venus'][-30:]}" + ) + assert max(abs(p) for p in power["b2500"][-30:]) < 1.0 + + +@pytest.mark.parametrize( + "dc_device_type", + [ + "HMA-X", + "HMB-X", + "HMJ-X", + "HMK-X", + "hma-x", + "JUPITER-1", + "UNKNOWN", + "", + ], +) +def test_non_venus_prefixes_are_treated_as_dc(dc_device_type: str) -> None: + """B2500 family, Jupiter, and anything unrecognised default to DC. - grid, power = _run_scenario([venus], surplus_watts=600.0, ticks=200) + Paired with a recognised Venus, these must be held at 0 W under + surplus while the Venus absorbs everything. Empty / unknown + device types share the same DC default (fail-closed policy). + """ + dc = DCOnlyBattery("dc", device_type=dc_device_type) + venus = ACBatteryWithStartupThreshold("venus", device_type="HMG-50") - venus_tail = power["venus_solo"][-30:] - grid_tail = grid[-30:] + _, power = _run_scenario([dc, venus], surplus_watts=600.0, ticks=200) - # Venus woke up and absorbed the surplus. - assert min(venus_tail) < -500, ( - f"Venus (solo) should absorb ~-600 W, tail was {venus_tail}" + assert max(abs(p) for p in power["dc"][-30:]) < 1.0, ( + f"device_type={dc_device_type!r} should be excluded from charge; " + f"tail was {power['dc'][-30:]}" + ) + assert min(power["venus"][-30:]) < -500, ( + f"Venus should absorb the surplus while the DC sibling is held at 0. " + f"Venus tail: {power['venus'][-30:]}" ) - avg_grid = sum(grid_tail) / len(grid_tail) - assert abs(avg_grid) < 30, f"Grid should converge near 0 W, got {avg_grid:.0f} W" -def test_discharging_still_works_with_mixed_batteries() -> None: - """Control: the user notes discharging works; verify it here. +# --------------------------------------------------------------------------- +# Discharge unaffected +# --------------------------------------------------------------------------- - With a positive grid import (house consuming more than solar), both - batteries can contribute — the B2500 by discharging normally, the - Venus likewise — so splitting the target evenly is fine. + +def test_dc_discharge_still_shared_with_ac_sibling() -> None: + """The gate is ``grid_total < 0`` only — imports still share across both. + + A B2500 + Venus pair facing 1 kW of house consumption should both + discharge (the B2500 can do that fine) and together drain the grid + to ~0. Protects the user's observation that discharging was + working with the original balancer. """ - b2500 = DCOnlyBattery("b2500_dc_dis") - venus = ACBatteryWithStartupThreshold("venus_ac_dis", startup_min=400.0) + b2500 = DCOnlyBattery("b2500", device_type="HMJ-1") + venus = ACBatteryWithStartupThreshold("venus", device_type="HMG-50") - # 1000 W of house consumption with no solar → grid imports 1000 W; - # ``_run_scenario`` uses ``-surplus_watts`` so we pass a negative - # "surplus" to simulate import. grid, power = _run_scenario([b2500, venus], surplus_watts=-1000.0, ticks=200) avg_grid = sum(grid[-30:]) / 30 - # Both batteries discharge ~500 W each — grid drained to ~0. assert abs(avg_grid) < 50, ( - f"Discharge across both batteries should drain grid, got {avg_grid:.0f} W" + f"Discharge across both batteries should drain the grid; got {avg_grid:.0f} W" ) - assert sum(power["b2500_dc_dis"][-30:]) / 30 > 400, ( - "B2500 should be actively discharging" + assert sum(power["b2500"][-30:]) / 30 > 400, "B2500 should be discharging" + assert sum(power["venus"][-30:]) / 30 > 400, "Venus should be discharging" + + +# --------------------------------------------------------------------------- +# Degenerate all-DC-under-surplus case +# --------------------------------------------------------------------------- + + +def test_all_dc_under_surplus_holds_zero_and_logs( + caplog: pytest.LogCaptureFixture, +) -> None: + """Two B2500s under surplus: nothing can absorb; log once and hold at 0. + + No recognised AC-chargeable battery is reporting, so the balancer + cannot do anything useful with the surplus. It should: + * hold every consumer at 0 W (no stray charge commands), + * surface an info-level notice listing the device_types it saw, + so the user can diagnose the mix. + The message is latched — only one line per transition into the + state, not one per tick. + """ + a = DCOnlyBattery("dc_a", device_type="HMJ-1") + b = DCOnlyBattery("dc_b", device_type="HMA-2") + + with caplog.at_level(logging.INFO, logger="astrameter"): + _, power = _run_scenario([a, b], surplus_watts=600.0, ticks=200) + + assert max(abs(p) for p in power["dc_a"]) < 1.0 + assert max(abs(p) for p in power["dc_b"]) < 1.0 + + dc_messages = [ + rec.getMessage() + for rec in caplog.records + if "no AC-chargeable battery" in rec.getMessage() + ] + assert len(dc_messages) == 1, ( + f"Expected exactly one latched warning, got {len(dc_messages)}: {dc_messages}" ) - assert sum(power["venus_ac_dis"][-30:]) / 30 > 400, ( - "Venus should also be discharging" + assert "600" in dc_messages[0], "Message should include the surplus magnitude" + + +# --------------------------------------------------------------------------- +# Legacy / regression protection for the original deadlock +# --------------------------------------------------------------------------- + + +def test_unknown_device_type_deadlock_persists() -> None: + """Protects the default-to-DC safety policy. + + The original bug reproduction used empty ``device_type`` strings: + both batteries look like unknown DC batteries to the balancer, so + nobody is asked to charge and the grid keeps feeding back the + surplus. This is the expected fail-closed behaviour for consumers + we can't identify. + """ + b2500 = DCOnlyBattery("b2500_legacy", device_type="") + venus = ACBatteryWithStartupThreshold("venus_legacy", device_type="") + + grid, power = _run_scenario([b2500, venus], surplus_watts=600.0, ticks=200) + + assert max(abs(p) for p in power["venus_legacy"][-30:]) < 1.0 + assert max(abs(p) for p in power["b2500_legacy"][-30:]) < 1.0 + avg_grid = sum(grid[-30:]) / 30 + assert avg_grid < -550, ( + f"With unknown device_types nobody charges; expected grid ~= -600 W, got " + f"{avg_grid:.0f} W" ) diff --git a/tests/test_ct002_active_control.py b/tests/test_ct002_active_control.py index 264a1a34..c6504e17 100644 --- a/tests/test_ct002_active_control.py +++ b/tests/test_ct002_active_control.py @@ -530,8 +530,11 @@ def test_negative_target_concentrates(self): min_efficient_power=150, efficiency_fade_alpha=1.0, ) - device._update_consumer_report("a", "A", 0) - device._update_consumer_report("b", "A", 0) + # Use an AC-chargeable device_type so the efficiency-concentration + # pipeline under test is not short-circuited by the DC-blind guard + # (see issue #338; AC_CHARGEABLE_DEVICE_PREFIXES in balancer.py). + device._update_consumer_report("a", "A", 0, device_type="HMG-50") + device._update_consumer_report("b", "A", 0, device_type="HMG-50") out_a = device._compute_smooth_target([-200, 0, 0], "a") out_b = device._compute_smooth_target([-200, 0, 0], "b") # One should get ~-200W, the other ~0W From a1cbe225f0b6f59fbec0febb0dfabf5739374259 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Apr 2026 05:58:01 +0000 Subject: [PATCH 3/4] Handle B2500 pass-through at 100% SoC (#338) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported by the repo owner as a follow-up: when the B2500 is at 100% SoC any incoming DC solar flows straight through as AC output, so it reports positive power regardless of commands. Against the pre-fix balancer this pins the Venus around -340 W via the balance-correction + sign-clamp interaction, leaving ~160 W of sustained feed-in — and reproduces the user's symptom without any Venus startup-threshold assumption. The earlier commit fixed the split-evenly deadlock but the strict ``grid_total < 0`` guard let go at the exact zero-crossing during pass-through equilibrium (Venus at -500 W, B2500 at +500 W, grid at 0 W): the balance-correction fired, pushed Venus positive by one max_correction_per_step, and the system oscillated. Extend the charge-blind mask to also fire at ``grid_total == 0`` when any AC-chargeable battery is currently charging (``power < 0``). That signals pass-through equilibrium specifically, and keeps the discharge path untouched — a pure-discharge equilibrium at grid=0 has both batteries at positive power, so the extension doesn't activate. Adds ``B2500PassThrough`` to the test harness and a regression test verifying Venus converges to -500 W and grid to 0 W under the 500 W pass-through scenario. --- src/astrameter/ct002/balancer.py | 28 +++++-- tests/test_balancer_mixed_battery_charging.py | 75 +++++++++++++++++++ 2 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/astrameter/ct002/balancer.py b/src/astrameter/ct002/balancer.py index 8a3da23c..ba564ab1 100644 --- a/src/astrameter/ct002/balancer.py +++ b/src/astrameter/ct002/balancer.py @@ -832,18 +832,34 @@ def _compute_auto_target( num_consumers = max(1, len(reports)) eff_part = {cid: max(0.01, 1.0 - saturation.get(cid, 0.0)) for cid in reports} - # Exclude DC-only batteries (B2500 family, Jupiter, anything not in - # AC_CHARGEABLE_DEVICE_PREFIXES) from charge distribution under - # surplus. They'll happily discharge on positive grid_total, but - # asking them to charge wastes the share that should have gone to - # the AC sibling (Venus). See issue #338. + # Exclude DC-only batteries (B2500 family, Jupiter, anything not + # in AC_CHARGEABLE_DEVICE_PREFIXES) from charge distribution + # whenever the grid is in charge territory. The base gate is + # ``grid_total < 0`` (surplus), but we also extend it to the + # exact zero-crossing when an AC-chargeable battery is already + # charging (``power < 0``) — that signals pass-through + # equilibrium, which happens when a full B2500 is passing its DC + # solar input through as AC output (+P W) while the Venus + # charges a matching -P W, leaving grid at 0. Without this + # extension the balance-correction fires at the zero-crossing + # and oscillates the Venus back out of its steady state. We + # deliberately don't fire on ``grid_total == 0`` during pure + # discharge (both batteries discharging to serve the house load) + # because no AC-chargeable battery is charging there. + # See issue #338. + ac_charging = any( + _is_ac_chargeable(r.get("device_type", "")) + and parse_int(r.get("power", 0)) < 0 + for r in reports.values() + ) + in_charge_territory = grid_total < 0 or (grid_total == 0 and ac_charging) charge_blind = ( { cid for cid, r in reports.items() if not _is_ac_chargeable(r.get("device_type", "")) } - if grid_total < 0 + if in_charge_territory else set() ) for cid in charge_blind: diff --git a/tests/test_balancer_mixed_battery_charging.py b/tests/test_balancer_mixed_battery_charging.py index d6987f3e..49e61759 100644 --- a/tests/test_balancer_mixed_battery_charging.py +++ b/tests/test_balancer_mixed_battery_charging.py @@ -140,6 +140,37 @@ def step(self, target_delta: float, reported_power: float) -> None: self.power = 0.0 +class B2500PassThrough: + """B2500 at 100 % SoC passing its DC solar input straight through as AC. + + When the B2500 is full it can no longer absorb its own DC input, so + the excess flows out as AC — the unit reports positive power + (apparent discharge) regardless of any CT command. The balancer + sees this as "B2500 is producing" while the real grid is still + importing the surplus it can't absorb. See issue #338 (follow-up + from the repo owner): this scenario reproduces the deadlock + *without* requiring any Venus startup-threshold assumption, because + the balance-correction + sign-clamp interaction alone pins the + Venus below the level needed to cancel the B2500's feed. + """ + + def __init__( + self, + mac: str, + passthrough_w: int, + *, + device_type: str = "HMJ-1", + ) -> None: + self.mac = mac + self.passthrough_w = passthrough_w + self.device_type = device_type + self.power = float(passthrough_w) + + def step(self, target_delta: float, reported_power: float) -> None: + # Output is dictated by DC solar input, not the CT command. + self.power = float(self.passthrough_w) + + def _make_balancer(clock: _FakeClock) -> LoadBalancer: """Balancer with CT002 defaults (matching the out-of-the-box config).""" return LoadBalancer( @@ -317,6 +348,50 @@ def test_non_venus_prefixes_are_treated_as_dc(dc_device_type: str) -> None: ) +# --------------------------------------------------------------------------- +# B2500 pass-through at 100 % SoC +# --------------------------------------------------------------------------- + + +def test_b2500_passthrough_at_full_soc_does_not_pin_venus() -> None: + """B2500 full + passing 500 W DC through as AC: Venus must absorb it. + + Without the fix the pre-fix balancer pins the Venus at ~-340 W: the + balance correction treats the B2500's +500 W "output" as a peer + behaviour the Venus should match toward, and the sign clamp then + blocks Venus from being pushed negative enough to cancel the feed. + The result is a sustained ~160 W export, independent of any + inverter startup threshold. + + With the fix, the B2500 is recognised by prefix (``HMJ-1``) as + DC-only and excluded from charge distribution; the Venus receives + the full -500 W target, charges to -500 W, and pins the grid at 0. + """ + b2500 = B2500PassThrough("b2500_full", passthrough_w=500) + venus = ACBatteryWithStartupThreshold("venus", device_type="HMG-50") + + grid, power = _run_scenario([b2500, venus], surplus_watts=0.0, ticks=60) + + venus_tail = power["venus"][-20:] + b2500_tail = power["b2500_full"][-20:] + grid_tail = grid[-20:] + + # B2500 keeps pushing its 500 W pass-through regardless of commands. + assert all(abs(p - 500.0) < 1.0 for p in b2500_tail), ( + f"B2500 pass-through output should stay at 500 W, tail was {b2500_tail}" + ) + # Venus absorbs the full pass-through; grid at ~0. + assert min(venus_tail) < -490, ( + f"Venus should converge near -500 W to cancel the pass-through, " + f"tail was {venus_tail}" + ) + avg_grid = sum(grid_tail) / len(grid_tail) + assert abs(avg_grid) < 30, ( + f"Grid should converge near 0 W (Venus exactly cancels B2500), " + f"got {avg_grid:.0f} W" + ) + + # --------------------------------------------------------------------------- # Discharge unaffected # --------------------------------------------------------------------------- From cc64a9a722b7524300e85daff2fe0a99ebc64f76 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Apr 2026 06:10:34 +0000 Subject: [PATCH 4/4] Tidy test averaging + cross-reference log assertion (#338) Review nitpicks: * test_dc_discharge_still_shared_with_ac_sibling now computes tail averages with ``len(slice)`` instead of a hard-coded ``/30``. * test_unknown_device_type_deadlock_persists docstring now notes that it also exercises the ``all_dc_under_surplus`` branch, with the log-latching behaviour asserted in the dedicated ``test_all_dc_under_surplus_holds_zero_and_logs`` test. --- tests/test_balancer_mixed_battery_charging.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/test_balancer_mixed_battery_charging.py b/tests/test_balancer_mixed_battery_charging.py index 49e61759..635c28c1 100644 --- a/tests/test_balancer_mixed_battery_charging.py +++ b/tests/test_balancer_mixed_battery_charging.py @@ -410,12 +410,15 @@ def test_dc_discharge_still_shared_with_ac_sibling() -> None: grid, power = _run_scenario([b2500, venus], surplus_watts=-1000.0, ticks=200) - avg_grid = sum(grid[-30:]) / 30 + tail_grid = grid[-30:] + tail_b2500 = power["b2500"][-30:] + tail_venus = power["venus"][-30:] + avg_grid = sum(tail_grid) / len(tail_grid) assert abs(avg_grid) < 50, ( f"Discharge across both batteries should drain the grid; got {avg_grid:.0f} W" ) - assert sum(power["b2500"][-30:]) / 30 > 400, "B2500 should be discharging" - assert sum(power["venus"][-30:]) / 30 > 400, "Venus should be discharging" + assert sum(tail_b2500) / len(tail_b2500) > 400, "B2500 should be discharging" + assert sum(tail_venus) / len(tail_venus) > 400, "Venus should be discharging" # --------------------------------------------------------------------------- @@ -468,7 +471,9 @@ def test_unknown_device_type_deadlock_persists() -> None: both batteries look like unknown DC batteries to the balancer, so nobody is asked to charge and the grid keeps feeding back the surplus. This is the expected fail-closed behaviour for consumers - we can't identify. + we can't identify. This also exercises the ``all_dc_under_surplus`` + branch; the one-shot info-log emitted there is asserted separately + by ``test_all_dc_under_surplus_holds_zero_and_logs``. """ b2500 = DCOnlyBattery("b2500_legacy", device_type="") venus = ACBatteryWithStartupThreshold("venus_legacy", device_type="")