From ea92725bcf6eef9f3b0d85da0ec33d566268eb29 Mon Sep 17 00:00:00 2001 From: bvweerd Date: Fri, 3 Jul 2026 17:56:23 +0000 Subject: [PATCH] fix(battery): stop scheduled batteries and the PID from double-correcting The scheduled-battery correction (filtered x capacity share, summed to the full grid error) and the PID both corrected the same grid error every cycle. With a combined loop gain of ~2 the closed loop diverged: a 1000 W import escalated into a sustained full-power oscillation (grid swinging between -7 kW and +8 kW, battery bang-banging between full charge and full discharge every 5 s). - The commanded-but-not-yet-measured scheduled battery response is now fed forward into the PID residual (same trust-window mechanism as reactive batteries), so the PID only handles what the battery will not absorb. Closed-loop regression test included. - SoC limits now also bound the corrected schedule target: no charging at/above max_soc, no discharging at/below min_soc. Previously the correction could push a full battery into charge or an empty one into discharge (SoC was only enforced in the reactive layer). - An unavailable/unknown/non-numeric schedule sensor no longer degrades silently to correction-only control without SoC protection; the battery falls back to the reactive layer (SoC-aware, incremental) and a warning is logged once until the sensor recovers. - Scheduled setpoint writes are skipped when the target is unchanged (< 1 W), matching the reactive layer and removing per-cycle service call spam. Known limitation (unchanged, by design): the proportional correction steers the grid toward zero, so a schedule that intentionally imports or exports (e.g. cheap night charging) is partially cancelled. Fixing that needs a grid-plan signal from the optimizer alongside the battery schedule. https://claude.ai/code/session_01RUWpwxbGsgR3PoLHLq4Djz --- .../zero_grid_controller/control_engine.py | 96 ++++++++-- tests/test_scheduled_battery.py | 176 ++++++++++++++++-- 2 files changed, 241 insertions(+), 31 deletions(-) diff --git a/custom_components/zero_grid_controller/control_engine.py b/custom_components/zero_grid_controller/control_engine.py index e4ff504..aff56c8 100644 --- a/custom_components/zero_grid_controller/control_engine.py +++ b/custom_components/zero_grid_controller/control_engine.py @@ -103,6 +103,7 @@ def __init__( self._settling_until: dict[str, float] = {} self._load_settling_until: dict[str, float] = {} self._battery_pending_until: dict[str, float] = {} + self._schedule_unavailable: set[str] = set() # ------------------------------------------------------------------ # Public API @@ -298,8 +299,33 @@ async def run_cycle( # 5. Reactive battery state (pure reads) and PV recovery bias. # Recovery adds a virtual import so curtailed arrays gradually reopen # while reactive batteries still have spare charge capacity. - scheduled_batteries = [b for b in batteries if b.schedule_sensor_entity] - reactive_batteries = [b for b in batteries if not b.schedule_sensor_entity] + # A scheduled battery whose optimizer sensor is unavailable falls back + # to the reactive layer (SoC-aware, incremental) instead of silently + # degrading to correction-only control. + scheduled_batteries: list[BatteryConfig] = [] + reactive_batteries: list[BatteryConfig] = [] + schedule_values: dict[str, float] = {} + for battery in batteries: + if not battery.schedule_sensor_entity: + reactive_batteries.append(battery) + continue + schedule_w = self.read_power_w(battery.schedule_sensor_entity) + if schedule_w is None: + if battery.name not in self._schedule_unavailable: + self._schedule_unavailable.add(battery.name) + _LOGGER.warning( + "Schedule sensor %s for battery %s is unavailable — " + "falling back to reactive control", + battery.schedule_sensor_entity, + battery.name, + ) + reactive_batteries.append(battery) + continue + if battery.name in self._schedule_unavailable: + self._schedule_unavailable.discard(battery.name) + _LOGGER.info("Schedule sensor for battery %s recovered", battery.name) + schedule_values[battery.name] = schedule_w + scheduled_batteries.append(battery) battery_state = self._read_battery_states(reactive_batteries) recovery_w = self._pv_recovery_w(arrays, battery_state, filtered, now) @@ -333,14 +359,11 @@ async def run_cycle( # 6. Scheduled batteries follow an external optimizer # (e.g. battery_controller) with a real-time grid correction on top. + scheduled_pending_w = 0.0 if scheduled_batteries: total_scheduled_cap = sum(b.max_charge_w for b in scheduled_batteries) for battery in scheduled_batteries: - schedule_sensor = battery.schedule_sensor_entity - assert ( - schedule_sensor is not None - ) # guaranteed by list comprehension above - schedule_w = self.read_power_w(schedule_sensor) or 0.0 + schedule_w = schedule_values[battery.name] # Proportional reactive correction: adjusts schedule up/down based on # the current grid error. filtered > 0 → importing → push toward # discharge (positive correction); filtered < 0 → exporting → push @@ -350,21 +373,53 @@ async def run_cycle( if total_scheduled_cap > 0 else 0.0 ) - target = clamp( - schedule_w + correction, - -battery.max_charge_w, - battery.max_discharge_w, + # SoC limits also bound the corrected target: no charging at + # or above max_soc, no discharging at or below min_soc. + soc = ( + self.read_sensor_safe(battery.soc_sensor_entity) + if battery.soc_sensor_entity + else None ) - try: - await self._actuators.write_numeric_entity( - battery.setpoint_entity, target - ) - except Exception: - _LOGGER.exception( - "Failed to write schedule setpoint for %s", battery.name - ) - else: + lower = ( + 0.0 + if soc is not None and soc >= battery.max_soc + else -battery.max_charge_w + ) + upper = ( + 0.0 + if soc is not None and soc <= battery.min_soc + else battery.max_discharge_w + ) + target = clamp(schedule_w + correction, lower, upper) + + actual = self.read_power_w(battery.sensor_entity) + if actual is None: + actual = self._current_battery_setpoints.get(battery.name, 0.0) + + previous = self._current_battery_setpoints.get(battery.name) + if ( + previous is None + or abs(target - previous) >= BATTERY_WRITE_THRESHOLD_W + ): + try: + await self._actuators.write_numeric_entity( + battery.setpoint_entity, target + ) + except Exception: + _LOGGER.exception( + "Failed to write schedule setpoint for %s", battery.name + ) + continue self._current_battery_setpoints[battery.name] = target + self._battery_pending_until[battery.name] = now + BATTERY_PENDING_S + + # Feed the commanded-but-not-yet-measured battery response + # forward so the PID does not correct the same grid error a + # second time (which oscillates: combined loop gain ≈ 2). + if now < self._battery_pending_until.get(battery.name, 0.0): + scheduled_pending_w += ( + self._current_battery_setpoints.get(battery.name, 0.0) - actual + ) # 6b. Reactive battery layer: incremental command, returns the grid # residual with the still-pending battery response fed forward so @@ -374,6 +429,7 @@ async def run_cycle( residual = await self._command_batteries( filtered, now, reactive_batteries, battery_state, arrays, loads ) + residual -= scheduled_pending_w residual += recovery_w # 7. PID + actuator distribution (skipped in one-sided idle modes) diff --git a/tests/test_scheduled_battery.py b/tests/test_scheduled_battery.py index 5716715..7f27e56 100644 --- a/tests/test_scheduled_battery.py +++ b/tests/test_scheduled_battery.py @@ -202,8 +202,9 @@ async def test_scheduled_battery_grid_correction_increases_charge_when_exporting # --------------------------------------------------------------------------- -async def test_scheduled_battery_sensor_unavailable_falls_back_to_correction_only(hass): - """When the schedule sensor is 'unavailable', schedule_w defaults to 0.""" +async def test_scheduled_battery_sensor_unavailable_falls_back_to_reactive(hass): + """An 'unavailable' schedule sensor drops the battery into the reactive + layer (SoC-aware, incremental) instead of correction-only control.""" entry = _entry(deadband_w=0.0, ewm_alpha=1.0) entry.add_to_hass(hass) coordinator = ZeroGridCoordinator(hass, entry) @@ -216,13 +217,13 @@ async def test_scheduled_battery_sensor_unavailable_falls_back_to_correction_onl result = await coordinator._async_update_data() sp = result.battery_setpoints.get("SchedBat") - # schedule_w = 0.0, correction = +400, so target = +400 (discharge to cover import) + # Reactive layer: import 400 W, no PV/loads to correct → discharge 400 W assert sp is not None assert sp > 0 -async def test_scheduled_battery_sensor_unknown_falls_back_to_correction_only(hass): - """When the schedule sensor is 'unknown', schedule_w defaults to 0.""" +async def test_scheduled_battery_sensor_unknown_falls_back_to_reactive(hass): + """An 'unknown' schedule sensor drops the battery into the reactive layer.""" entry = _entry(deadband_w=0.0, ewm_alpha=1.0) entry.add_to_hass(hass) coordinator = ZeroGridCoordinator(hass, entry) @@ -236,13 +237,13 @@ async def test_scheduled_battery_sensor_unknown_falls_back_to_correction_only(ha sp = result.battery_setpoints.get("SchedBat") assert sp is not None - assert sp > 0 # correction only — grid importing, push toward discharge + assert sp > 0 # reactive discharge — grid importing, nothing else to correct -async def test_scheduled_battery_sensor_entity_missing_falls_back_to_correction_only( +async def test_scheduled_battery_sensor_entity_missing_falls_back_to_reactive( hass, ): - """When the schedule sensor entity does not exist at all, schedule_w defaults to 0.""" + """A missing schedule sensor entity drops the battery into the reactive layer.""" entry = _entry(deadband_w=0.0, ewm_alpha=1.0) entry.add_to_hass(hass) coordinator = ZeroGridCoordinator(hass, entry) @@ -258,11 +259,11 @@ async def test_scheduled_battery_sensor_entity_missing_falls_back_to_correction_ sp = result.battery_setpoints.get("SchedBat") assert sp is not None - assert sp > 0 # correction only + assert sp > 0 # reactive discharge -async def test_scheduled_battery_sensor_non_numeric_falls_back_to_correction_only(hass): - """A non-numeric sensor state (e.g. 'error') is treated as schedule_w = 0.""" +async def test_scheduled_battery_sensor_non_numeric_falls_back_to_reactive(hass): + """A non-numeric schedule sensor state drops the battery into the reactive layer.""" entry = _entry(deadband_w=0.0, ewm_alpha=1.0) entry.add_to_hass(hass) coordinator = ZeroGridCoordinator(hass, entry) @@ -508,3 +509,156 @@ async def capture_write(entity_id, value): assert abs(sp_b - 600.0) < 5.0 # BatB correction is 3× BatA correction assert abs(sp_b / sp_a - 3.0) < 0.1 + + +# --------------------------------------------------------------------------- +# Closed loop: scheduled battery + PID must not double-correct +# --------------------------------------------------------------------------- + + +async def test_scheduled_battery_and_pid_do_not_oscillate(hass): + """The scheduled correction is fed forward into the PID residual. + + Without the feedforward, the scheduled battery and the PID each corrected + the full grid error every cycle (combined loop gain ≈ 2) and the closed + loop diverged into a full-power charge/discharge oscillation. + """ + from custom_components.zero_grid_controller.array import ArrayConfig + + entry = _entry(deadband_w=10.0, ewm_alpha=1.0, kp=1.0, ki=0.0) + entry.add_to_hass(hass) + coordinator = ZeroGridCoordinator(hass, entry) + coordinator.arrays = [ + ArrayConfig( + name="Solar", + output_type="percent", + setpoint_entity="number.solar", + w_per_unit=50.0, + calibration_confidence="estimated", + setpoint_min=0.0, + setpoint_max=100.0, + settling_time_s=0, + ) + ] + coordinator.batteries = [_make_scheduled_battery()] + coordinator._engine._current_setpoints["Solar"] = 40.0 # limited to 2000 W + + plant = {"sp": 40.0, "bat": 0.0, "bat_target": 0.0} + clock = {"t": 1000.0} + + class _FakeTime: + @staticmethod + def monotonic() -> float: + return clock["t"] + + def publish() -> float: + pv = min(5000.0, plant["sp"] * 50.0) + grid = 3000.0 - pv - plant["bat"] + _grid(hass, import_w=max(0.0, grid), export_w=max(0.0, -grid)) + hass.states.async_set("sensor.battery_power", str(plant["bat"])) + hass.states.async_set("sensor.optimizer_setpoint", "0") + return grid + + async def on_sp(array, value): + plant["sp"] = value + + async def on_num(entity_id, value): + if entity_id == "number.battery_sp": + plant["bat_target"] = value + + publish() + grid_trace: list[float] = [] + with ( + patch("custom_components.zero_grid_controller.control_engine.time", _FakeTime), + patch.object( + coordinator._actuators, + "write_setpoint", + new=AsyncMock(side_effect=on_sp), + ), + patch.object( + coordinator._actuators, + "write_numeric_entity", + new=AsyncMock(side_effect=on_num), + ), + ): + for _ in range(15): + clock["t"] += 5.0 + await coordinator._async_update_data() + plant["bat"] = plant["bat_target"] # battery follows instantly + grid_trace.append(publish()) + + # Converged, not oscillating: the last cycles stay within the deadband + assert all(abs(g) <= 10.0 for g in grid_trace[-5:]), grid_trace + + +# --------------------------------------------------------------------------- +# SoC limits bound the corrected schedule target +# --------------------------------------------------------------------------- + + +async def test_scheduled_battery_soc_max_blocks_charging(hass): + """At/above max SoC the corrected target is clamped to ≥ 0 (no charge).""" + entry = _entry(deadband_w=0.0, ewm_alpha=1.0) + entry.add_to_hass(hass) + coordinator = ZeroGridCoordinator(hass, entry) + battery = _make_scheduled_battery() + battery.soc_sensor_entity = "sensor.battery_soc" + battery.max_soc = 95.0 + coordinator.batteries = [battery] + + hass.states.async_set("sensor.battery_soc", "96") + hass.states.async_set("sensor.optimizer_setpoint", "-2000") # optimizer: charge + _grid(hass, import_w=0, export_w=500) + + with patch.object(coordinator._actuators, "write_numeric_entity", new=AsyncMock()): + result = await coordinator._async_update_data() + + sp = result.battery_setpoints.get("SchedBat") + assert sp is not None + assert sp >= 0, "Full battery must not be commanded to charge" + + +async def test_scheduled_battery_soc_min_blocks_discharge(hass): + """At/below min SoC the corrected target is clamped to ≤ 0 (no discharge).""" + entry = _entry(deadband_w=0.0, ewm_alpha=1.0) + entry.add_to_hass(hass) + coordinator = ZeroGridCoordinator(hass, entry) + battery = _make_scheduled_battery() + battery.soc_sensor_entity = "sensor.battery_soc" + battery.min_soc = 10.0 + coordinator.batteries = [battery] + + hass.states.async_set("sensor.battery_soc", "8") + hass.states.async_set("sensor.optimizer_setpoint", "2000") # optimizer: discharge + _grid(hass, import_w=500, export_w=0) + + with patch.object(coordinator._actuators, "write_numeric_entity", new=AsyncMock()): + result = await coordinator._async_update_data() + + sp = result.battery_setpoints.get("SchedBat") + assert sp is not None + assert sp <= 0, "Empty battery must not be commanded to discharge" + + +async def test_scheduled_battery_write_skipped_when_unchanged(hass): + """An unchanged scheduled target (< 1 W delta) is not rewritten every cycle.""" + entry = _entry(deadband_w=0.0, ewm_alpha=1.0) + entry.add_to_hass(hass) + coordinator = ZeroGridCoordinator(hass, entry) + coordinator.batteries = [_make_scheduled_battery()] + + hass.states.async_set("sensor.optimizer_setpoint", "-1500") + hass.states.async_set("sensor.battery_power", "-1500") + _grid(hass, import_w=0, export_w=0) + hass.states.async_set("sensor.grid_import", "0.4") # tiny error < 1 W target delta + + with patch.object( + coordinator._actuators, "write_numeric_entity", new=AsyncMock() + ) as mock_write: + await coordinator._async_update_data() + first_calls = mock_write.await_count + await coordinator._async_update_data() + + assert mock_write.await_count == first_calls, ( + "Unchanged scheduled target must not be rewritten" + )