Skip to content

Design: stop recreating InverterController every optimization cycle #369

Description

@johanzander

Background

Found while fixing #329 (VPP status/allow-AC-charging flash registers rewritten on every schedule change instead of once).

BatterySystemManager._create_updated_schedule builds a brand-new temp_growatt = self._create_inverter_controller() every optimization cycle, computes the new schedule into it via create_schedule(), and then either discards it or adopts it as self._inverter_controller (_apply_schedule/_should_apply_schedule branch, battery_system_manager.py).

The problem: InverterController subclasses hold two kinds of state that don't belong in the same object lifecycle:

  1. Hardware-observed/confirmed state — genuinely persistent, tied to the physical inverter connection, and must survive across cycles: _vpp_status_confirmed, _last_written_vpp_remote_control, _last_written_vpp_power, _last_written_tou_mode, tou_intervals, _active_tou_intervals.
  2. Per-cycle schedule content — transient, meant to be recomputed fresh every cycle: strategic_intents, the computed TOU intervals for the new candidate schedule.

Because a fresh controller instance is built every cycle to get (2) recomputed, (1) has to be explicitly copied forward or it silently resets to its __init__ default — which is exactly what caused #329, and a second latent instance of the same bug shape found for _last_written_tou_mode during the same investigation.

What #329's fix did (band-aid, not a cure)

Added InverterController.seed_from(other), overridden per subclass to enumerate every hardware-derived field once, called unconditionally right after each temp_growatt is created (PR #368). This consolidates the carry-forward into one place instead of leaving it scattered across call sites — but it's still a manually-maintained list that must be updated every time a new piece of hardware-observed state is added. It had already drifted out of sync once before this fix (the pre-#329 carry-forward only covered TOU intervals, missing the VPP fields added later and _last_written_tou_mode). seed_from makes the next drift instance easier to fix, but doesn't prevent it.

Proposed direction

Stop recreating the controller at all. Keep a single long-lived InverterController instance for the lifetime of a given hardware connection/platform selection, and:

  • Mutate its schedule-related fields (strategic_intents, computed TOU intervals) in place each cycle, rather than building a scratch instance.
  • Base the "should we apply this schedule" comparison (_should_apply_schedule/compare_schedules) on comparing schedule data (e.g. two DPSchedule/intent-list values) rather than comparing two controller instances.
  • Never touch the hardware-observed-state fields as part of a schedule update — they'd only ever change via an explicit hardware read-back (read_and_initialize_from_hardware) or an explicit write confirmation.

This removes the whole bug class (no more "did we remember to carry this field forward") instead of patching around it, at the cost of a real refactor across battery_system_manager.py's schedule-apply path and all 4 InverterController subclasses (GrowattMinController, GrowattSphController, SolaxController, SolaxModbusGrowattController).

Also worth checking while in there

Code review during #329 flagged that growatt_sph_controller.py (_charge_periods/_discharge_periods) and solax_controller.py weren't audited for the same carry-forward gap — not confirmed as broken, but worth checking as part of whatever refactor lands here.

Scope

Design/refactor, no user-facing bug on its own — file as a design-debt issue, not urgent. Related: #329, PR #368.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions