hybrid system: fix consideration of max out power by LKuemmel · Pull Request #3247 · openWB/core

LKuemmel · 2026-03-27T08:41:16Z

Ticket #66002733

Copilot

Pull request overview

This PR (Ticket #66002733) adjusts the hybrid-system battery power limiting logic to better respect PV inverter max_ac_out constraints, and updates unit tests accordingly.

Changes:

Renames _inverter_limited_power to _get_pv_power_beyond_max_ac_out and updates call sites.
Changes _limit_bat_power_discharge to clamp requested power based on PV power exceeding max_ac_out and current battery power.
Updates/extends unit tests to match the renamed helper and the new limiting behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
packages/control/bat_all.py	Renames the helper and changes the limiting formula for hybrid max-AC-out behavior.
packages/control/bat_all_test.py	Updates tests for the renamed helper and adds new parametrized cases for the updated limiting logic.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T08:46:59Z

+            required_power = max(min(required_power, max_inverter_power_for_bat), 0)
+            log.debug(f"Verbleibende Speicher-Leistung durch maximale Ausgangsleistung auf {required_power}W"
+                      " begrenzt.")


In _limit_bat_power_discharge, max_inverter_power_for_bat = self.data.get.power - pv_power_beyond_max_ac_out can become negative when the battery is currently discharging (self.data.get.power < 0). Since required_power can still be > 0 in that situation (e.g. _get_charging_power_left computes charging_power_left = bat_power_discharge + self.data.get.power), this will clamp required_power to 0 even when pv_power_beyond_max_ac_out is 0, which likely prevents using remaining allowed discharge headroom. Consider basing the clamp on the meaning of required_power in both scenarios (battery charging vs discharging), e.g. handle self.data.get.power < 0 separately so required_power isn’t forced to 0 just because current battery power is negative.

Suggested change

required_power = max(min(required_power, max_inverter_power_for_bat), 0)

log.debug(f"Verbleibende Speicher-Leistung durch maximale Ausgangsleistung auf {required_power}W"

" begrenzt.")

# Wenn max_inverter_power_for_bat nur deshalb negativ ist, weil der Speicher aktuell bereits entlädt

# (self.data.get.power < 0) und keine PV-Leistung über der maximalen WR-Ausgangsleistung anliegt

# (pv_power_beyond_max_ac_out == 0), dann würde eine Begrenzung auf 0W die noch verfügbare

# Entladeleistung fälschlicherweise unterdrücken. In diesem Fall wenden wir keine zusätzliche

# Begrenzung durch die WR-Ausgangsleistung an.

if not (max_inverter_power_for_bat < 0 and self.data.get.power < 0 and pv_power_beyond_max_ac_out == 0):

required_power = max(min(required_power, max_inverter_power_for_bat), 0)

log.debug(

f"Verbleibende Speicher-Leistung durch maximale Ausgangsleistung auf {required_power}W begrenzt."

)

else:

log.debug(

"Speicher-Entladeleistung nicht durch maximale WR-Ausgangsleistung begrenzt, da der "

"Speicher bereits entlädt und keine PV-Leistung über der maximalen WR-Ausgangsleistung anliegt."

)

Copilot · 2026-03-27T08:46:59Z

+    def _get_pv_power_beyond_max_ac_out(self, inverter: Pv) -> float:
        """gibt die maximale Entladeleistung des Speichers zurück, bis die maximale Ausgangsleistung des WR erreicht
        ist."""
        # tested


The docstring for _get_pv_power_beyond_max_ac_out says it returns “die maximale Entladeleistung des Speichers …”, but the implementation returns PV power that exceeds max_ac_out (based on inverter.data.get.power and max_ac_out). Updating the docstring (and optionally the inline comment) to describe the returned value will avoid confusion for future maintenance.

Copilot · 2026-03-27T08:47:00Z

    # setup
    data.data.pv_data = {"pv2": Pv(2)}
-    mock_inverter_limited_power = Mock(return_value=return_inverter_limited_power)
-    monkeypatch.setattr(BatAll, "_inverter_limited_power", mock_inverter_limited_power)
+    mock_pv_power_beyond_max_ac_out = Mock(return_value=return_pv_power_beyond_max_ac_out)
+    monkeypatch.setattr(BatAll, "_get_pv_power_beyond_max_ac_out", mock_pv_power_beyond_max_ac_out)


data.data.pv_data = {"pv2": Pv(2)} assumes the global control.data.data has been initialized via data.data_init(...), but this test doesn’t request the local data_fixture (and doesn’t take the shared data_ fixture either). That makes the test order-dependent and it can fail in isolation. Include a fixture that calls data.data_init(...) in this test’s signature (or make the module’s data_fixture autouse).

Copilot · 2026-03-27T08:47:00Z

+        pytest.param(1000, 1000, 1100, 0, id="maximale Entladeleistung erreicht"),
+        pytest.param(3000, 3000, 2000, 1000, id="max Leistung des WR um 2000W überschritten"),
+        pytest.param(3000, 5000, 2000, 1000, id="max Leistung des WR um 2000W überschritten, " +
+                     "erlaubte Entladeleistung höher als aktuelle Leistung"),


test_limit_bat_power_discharge currently only exercises b.data.get.power values >= 0. _limit_bat_power_discharge can be invoked with required_power > 0 even when b.data.get.power < 0 (battery discharging but still with remaining allowed discharge headroom), and the new clamp logic is sensitive to that sign. Please add at least one parametrized case covering bat_power < 0 with required_power > 0 to guard against regressions.

Suggested change

"erlaubte Entladeleistung höher als aktuelle Leistung"),

"erlaubte Entladeleistung höher als aktuelle Leistung"),

pytest.param(-1000, 1000, 2000, 0, id="Speicher lädt, Entladung angefordert, PV bereits über max_ac_out"),

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

packages/control/bat_all_test.py:26

data_fixture is now autouse=True and calls data.data_init(...). Several tests in this module also request the data_ fixture from packages/conftest.py, which also calls data.data_init(...) and populates a different test dataset. Having two independent initializations in the same test can overwrite state in an order that pytest does not guarantee, leading to flaky or incorrect tests. Prefer making this fixture non-autouse and explicitly depend on it only in the tests that need it, or refactor so it composes with data_ instead of reinitializing global data twice.

@pytest.fixture(autouse=True)
def data_fixture() -> None:
    data.data_init(Mock())
    data.data.general_data = General()
    data.data.cp_all_data = Mock(spec=AllChargepoints, data=Mock(
        spec=AllChargepointData, get=Mock(spec=AllGet, power=0)))
    data.data.pv_data["pv1"] = Mock(spec=Pv, data=Mock(spec=PvData, get=Mock(spec=Get, power=-6400),
                                                       config=Mock(spec=Config, max_ac_out=7200)))

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T09:47:10Z

+            max_inverter_power_for_bat = self.data.get.power - pv_power_beyond_max_ac_out
+            # Wenn max_inverter_power_for_bat nur deshalb negativ ist, weil der Speicher aktuell bereits entlädt
+            # (self.data.get.power < 0) und keine PV-Leistung über der maximalen WR-Ausgangsleistung anliegt
+            # (pv_power_beyond_max_ac_out == 0), dann würde eine Begrenzung auf 0W die noch verfügbare
+            # Entladeleistung fälschlicherweise unterdrücken. In diesem Fall wenden wir keine zusätzliche
+            # Begrenzung durch die WR-Ausgangsleistung an.
+            if not (max_inverter_power_for_bat < 0 and self.data.get.power < 0 and pv_power_beyond_max_ac_out == 0):
+                required_power = max(min(required_power, max_inverter_power_for_bat), 0)
+                log.debug(
+                    f"Verbleibende Speicher-Leistung durch maximale Ausgangsleistung auf {required_power}W begrenzt."
+                )
+            else:
+                log.debug(
+                    "Speicher-Entladeleistung nicht durch maximale WR-Ausgangsleistung begrenzt, da der "
+                    "Speicher bereits entlädt und keine PV-Leistung über der maximalen WR-Ausgangsleistung anliegt."


In _limit_bat_power_discharge, max_inverter_power_for_bat = self.data.get.power - pv_power_beyond_max_ac_out will clamp required_power to <= self.data.get.power (after the min(...)). If self.data.get.power is 0 (battery currently idle), any positive required_power will be forced to 0 even when no inverter limit is active (pv_power_beyond_max_ac_out == 0). That prevents allowing additional discharge headroom and looks like a regression vs the previous behavior of only reducing by the WR saturation amount. Consider basing the limit on inverter headroom (e.g., derived from max_ac_out and current inverter power) or at least not capping by self.data.get.power when it’s 0/positive. Please also add a test case covering bat_power=0, required_power>0, pv_power_beyond_max_ac_out=0.

Suggested change

max_inverter_power_for_bat = self.data.get.power - pv_power_beyond_max_ac_out

# Wenn max_inverter_power_for_bat nur deshalb negativ ist, weil der Speicher aktuell bereits entlädt

# (self.data.get.power < 0) und keine PV-Leistung über der maximalen WR-Ausgangsleistung anliegt

# (pv_power_beyond_max_ac_out == 0), dann würde eine Begrenzung auf 0W die noch verfügbare

# Entladeleistung fälschlicherweise unterdrücken. In diesem Fall wenden wir keine zusätzliche

# Begrenzung durch die WR-Ausgangsleistung an.

if not (max_inverter_power_for_bat < 0 and self.data.get.power < 0 and pv_power_beyond_max_ac_out == 0):

required_power = max(min(required_power, max_inverter_power_for_bat), 0)

log.debug(

f"Verbleibende Speicher-Leistung durch maximale Ausgangsleistung auf {required_power}W begrenzt."

)

else:

log.debug(

"Speicher-Entladeleistung nicht durch maximale WR-Ausgangsleistung begrenzt, da der "

"Speicher bereits entlädt und keine PV-Leistung über der maximalen WR-Ausgangsleistung anliegt."

if pv_power_beyond_max_ac_out > 0:

max_inverter_power_for_bat = self.data.get.power - pv_power_beyond_max_ac_out

# Negative Werte bedeuten, dass bereits mehr Leistung über den WR fließt, als für den Speicher

# zusätzlich verfügbar ist; in diesem Fall ist keine weitere Entladung möglich.

max_inverter_power_for_bat = max(max_inverter_power_for_bat, 0)

required_power = min(required_power, max_inverter_power_for_bat)

log.debug(

f"Verbleibende Speicher-Leistung durch maximale Ausgangsleistung auf {required_power}W begrenzt."

)

else:

log.debug(

"Speicher-Entladeleistung nicht durch maximale WR-Ausgangsleistung begrenzt, da keine PV-Leistung "

"über der maximalen WR-Ausgangsleistung anliegt."

Copilot · 2026-03-27T09:47:11Z

-        ist."""
+    def _get_pv_power_beyond_max_ac_out(self, inverter: Pv) -> float:
+        """gibt die PV-Leistung zurück, die über der maximalen Ausgangsleistung des Wechselrichters liegt und somit
+        nicht für die Entadung des Speichers genutzt werden kann."""


Docstring contains a typo: "Entadung" should be "Entladung".

Suggested change

nicht für die Entadung des Speichers genutzt werden kann."""

nicht für die Entladung des Speichers genutzt werden kann."""

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T10:02:28Z

+@pytest.fixture(autouse=True)
 def data_fixture() -> None:
    data.data_init(Mock())
    data.data.general_data = General()


In diesem Testmodul gibt es bereits das data_-Fixture aus packages/conftest.py, das ebenfalls data.data_init(...) aufruft. Das neue autouse=True-Fixture initialisiert data damit für jeden Test doppelt (für Tests mit data_) und macht die Ausführungsreihenfolge der Fixtures relevant. Besser: dieses Fixture nicht als autouse verwenden, sondern data_ in den betroffenen Tests (z.B. test_pv_power_beyond_max_ac_out, test_limit_bat_power_discharge) explizit anfordern oder data_fixture von data_ abhängig machen und kein eigenes data.data_init(...) mehr ausführen.

Copilot · 2026-03-27T10:02:29Z

+        """gibt die PV-Leistung zurück, die über der maximalen Ausgangsleistung des Wechselrichters liegt und somit
+        nicht für die Entladung des Speichers genutzt werden kann."""


Im Docstring ist ein Tippfehler: „Entadung“ sollte „Entladung“ heißen.

Copilot · 2026-03-27T10:02:29Z

+            max_inverter_power_for_bat = self.data.get.power - pv_power_beyond_max_ac_out
+            # Wenn max_inverter_power_for_bat nur deshalb negativ ist, weil der Speicher aktuell bereits entlädt
+            # (self.data.get.power < 0) und keine PV-Leistung über der maximalen WR-Ausgangsleistung anliegt
+            # (pv_power_beyond_max_ac_out == 0), dann würde eine Begrenzung auf 0W die noch verfügbare
+            # Entladeleistung fälschlicherweise unterdrücken. In diesem Fall wenden wir keine zusätzliche
+            # Begrenzung durch die WR-Ausgangsleistung an.
+            if pv_power_beyond_max_ac_out > 0:
+                max_inverter_power_for_bat = self.data.get.power - pv_power_beyond_max_ac_out
+                # Negative Werte bedeuten, dass bereits mehr Leistung über den WR fließt, als für den Speicher


max_inverter_power_for_bat wird vor der if pv_power_beyond_max_ac_out > 0:-Abfrage berechnet (Zeile 204) und innerhalb des Blocks direkt nochmal identisch gesetzt (Zeile 211). Die erste Zuweisung ist damit redundant und kann entfernt werden, um die Logik klarer zu halten.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

packages/control/bat_all_test.py:25

The autouse data_fixture no longer calls data.data_init(...). In this codebase control.data.data is only created via data_init, so accessing data.data.general_data / data.data.cp_all_data here will raise at runtime for tests that don't request the data_ fixture (e.g. test_pv_power_beyond_max_ac_out). Reintroduce data.data_init(Mock()) in this fixture, or make the fixture depend on data_ (or another init fixture) to ensure data.data exists before mutating it.

@pytest.fixture(autouse=True)
def data_fixture() -> None:
    data.data.general_data = General()
    data.data.cp_all_data = Mock(spec=AllChargepoints, data=Mock(
        spec=AllChargepointData, get=Mock(spec=AllGet, power=0)))
    data.data.pv_data["pv1"] = Mock(spec=Pv, data=Mock(spec=PvData, get=Mock(spec=Get, power=-6400),
                                                       config=Mock(spec=Config, max_ac_out=7200)))

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

hybrid system: fix consideration of max out power

7022e9a

LKuemmel requested a review from Copilot March 27, 2026 08:41

Copilot started reviewing on behalf of LKuemmel March 27, 2026 08:41 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

review

e513fb8

LKuemmel requested a review from Copilot March 27, 2026 09:42

Copilot started reviewing on behalf of LKuemmel March 27, 2026 09:42 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

LKuemmel added 2 commits March 27, 2026 10:54

review

49899a9

typo

7573986

LKuemmel requested a review from Copilot March 27, 2026 09:55

Copilot started reviewing on behalf of LKuemmel March 27, 2026 09:56 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

review

2f3f3ec

LKuemmel requested a review from Copilot March 27, 2026 10:35

Copilot started reviewing on behalf of LKuemmel March 27, 2026 10:36 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

LKuemmel merged commit 74b6d23 into openWB:master Mar 27, 2026
5 checks passed

	"erlaubte Entladeleistung höher als aktuelle Leistung"),
	"erlaubte Entladeleistung höher als aktuelle Leistung"),
	pytest.param(-1000, 1000, 2000, 0, id="Speicher lädt, Entladung angefordert, PV bereits über max_ac_out"),

	nicht für die Entadung des Speichers genutzt werden kann."""
	nicht für die Entladung des Speichers genutzt werden kann."""

		"""gibt die PV-Leistung zurück, die über der maximalen Ausgangsleistung des Wechselrichters liegt und somit
		nicht für die Entladung des Speichers genutzt werden kann."""

Conversation

LKuemmel commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants