Limit battery charge rate by MaStr · Pull Request #260 · MaStr/batcontrol

MaStr · 2026-02-05T19:12:20Z

Create a new operation mode "limit battery charge rate".
This enables batcontrol to send a MAX_CHARGE Command to the inverter to avoid charging the battery at maximum available pv power.

Reason: Implementing peak shaving

Copilot

Pull request overview

This pull request introduces a new operation mode "MODE_LIMIT_BATTERY_CHARGE_RATE" (mode 8) to enable peak shaving by limiting the battery charge rate from PV sources. The feature allows batcontrol to send a MAX_CHARGE command to the inverter to avoid charging the battery at maximum available PV power.

Changes:

Added MODE_LIMIT_BATTERY_CHARGE_RATE constant and implementation across core, inverter interface, and all inverter implementations
Extended MQTT API with limit_battery_charge_rate topic for external control
Added comprehensive test coverage for the new mode including core functionality and inverter-specific implementations
Updated configuration with new parameters: min_pv_charge_rate and max_pv_charge_rate documentation

Reviewed changes

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

Show a summary per file

File	Description
src/batcontrol/core.py	Added MODE_LIMIT_BATTERY_CHARGE_RATE mode, limit_battery_charge_rate method, API methods for setting/getting limit, and integration with inverter control flow
src/batcontrol/inverter/inverter_interface.py	Added abstract method set_mode_limit_battery_charge to interface
src/batcontrol/inverter/fronius.py	Implemented set_mode_limit_battery_charge using TimeOfUse CHARGE_MAX rules with validation
src/batcontrol/inverter/mqtt_inverter.py	Implemented set_mode_limit_battery_charge publishing mode and limit to MQTT command topics
src/batcontrol/inverter/dummy.py	Added dummy implementation and modified refresh_api_values/shutdown methods
src/batcontrol/inverter/resilient_wrapper.py	Added resilience wrapper for set_mode_limit_battery_charge method
src/batcontrol/mqtt_api.py	Added publish_limit_battery_charge_rate method and updated documentation with mode 8
config/batcontrol_config_dummy.yaml	Documented min_pv_charge_rate and max_pv_charge_rate parameters with mode 8 usage
tests/batcontrol/test_core.py	New comprehensive test suite for MODE_LIMIT_BATTERY_CHARGE_RATE functionality
tests/batcontrol/test_production_offset.py	Fixed whitespace issues and updated mocks for InverterControlSettings compatibility
tests/batcontrol/inverter/test_*.py	Added tests for set_mode_limit_battery_charge in all inverter implementations

Copilot · 2026-02-05T19:17:23Z

tests/batcontrol/test_production_offset.py

                # Call run to apply the offset
                batcontrol.run()
-                
+
                # Check that production was offset correctly
                # Note: production[0] is adjusted for elapsed time in current interval
                # so we only check indices [1] and [2] for exact values
-                assert batcontrol.last_production is not None
-                # Check that offset was applied (values should be roughly half)
-                assert batcontrol.last_production[1] == pytest.approx(1000, rel=0.01)
-                assert batcontrol.last_production[2] == pytest.approx(1500, rel=0.01)
-                # For [0], just check it's less than original
-                assert batcontrol.last_production[0] < 500  # Should be ~500 or less due to elapsed time
-
-    def test_production_offset_api_set_valid(self, mock_config):
-        """Test setting production offset via API with valid value"""
-        with patch('batcontrol.core.tariff_factory'), \
-             patch('batcontrol.core.inverter_factory'), \
-             patch('batcontrol.core.solar_factory'), \
-             patch('batcontrol.core.consumption_factory'):
-
-            batcontrol = Batcontrol(mock_config)
-
-            # Set via API
-            batcontrol.api_set_production_offset(0.7)
-
-            # Should be updated
-            assert batcontrol.production_offset_percent == 0.7
-
-    def test_production_offset_api_set_invalid_negative(self, mock_config):
-        """Test setting production offset via API with invalid negative value"""
-        with patch('batcontrol.core.tariff_factory'), \
-             patch('batcontrol.core.inverter_factory'), \
-             patch('batcontrol.core.solar_factory'), \
-             patch('batcontrol.core.consumption_factory'):
-
+
            batcontrol = Batcontrol(mock_config)
            original_value = batcontrol.production_offset_percent
-            
+
            # Try to set invalid value
            batcontrol.api_set_production_offset(-0.5)
-            
+
            # Should not be updated
            assert batcontrol.production_offset_percent == original_value


The test method 'test_production_offset_applied_to_forecast' is incomplete. After line 164, the test should include assertions to verify that the production offset was correctly applied (e.g., checking batcontrol.last_production values). Instead, lines 166-173 contain unrelated test logic for testing invalid negative values in api_set_production_offset, which appears to be remnants from the deleted 'test_production_offset_api_set_valid' and 'test_production_offset_api_set_invalid_negative' test methods. This creates a broken test that doesn't properly validate the production offset functionality and includes misplaced test logic.

src/batcontrol/core.py

src/batcontrol/inverter/dummy.py

Copilot · 2026-02-05T19:17:24Z

tests/batcontrol/inverter/test_fronius_ids.py


 if __name__ == '__main__':
    unittest.main()
+


There's an extra blank line at the end of the file (line 472). According to PEP8 standards mentioned in the repository guidelines, source files should end with a single newline character, not multiple blank lines. This extra blank line should be removed.

Copilot · 2026-02-05T19:17:24Z

tests/batcontrol/test_core.py

+import pytest
+import sys
+import os
+from unittest.mock import Mock, MagicMock, patch


Import of 'Mock' is not used.

Suggested change

from unittest.mock import Mock, MagicMock, patch

from unittest.mock import MagicMock, patch

tests/batcontrol/test_core.py

- Added `min_pv_charge_rate` and `max_pv_charge_rate` to inverter configuration. - Introduced `MODE_LIMIT_BATTERY_CHARGE_RATE` to allow limiting PV charging while permitting battery discharge. - Updated `Batcontrol` class to handle new charge rate limits and modes. - Implemented `limit_battery_charge_rate` method in `Batcontrol` to apply dynamic limits based on configuration. - Enhanced `Dummy` and `FroniusWR` inverter classes to support the new limit battery charge mode. - Updated MQTT publishing to include commands for setting battery charge limits. - Added unit tests for the new functionality, including edge cases for charge limits.

- Add assertions to test_production_offset_applied_to_forecast to verify that last_production[1] and [2] reflect the 50% offset, and remove misplaced test logic that was left over from deleted test methods - Add warning log in api_set_mode when MODE_LIMIT_BATTERY_CHARGE_RATE is used but _limit_battery_charge_rate is still -1 (no limit set), so callers are informed about the silent fallback to allow-discharging - Remove extra blank line at end of tests/batcontrol/inverter/test_fronius_ids.py - Remove unused Mock import from tests/batcontrol/test_core.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

tests/batcontrol/test_production_offset.py

src/batcontrol/inverter/inverter_interface.py

Copilot · 2026-03-10T15:15:01Z

config/batcontrol_config_dummy.yaml

+  min_pv_charge_rate: 0 # Watt, STATIC lower limit for MODE_LIMIT_BATTERY_CHARGE_RATE (8).
+                        # Set to 0 to allow complete charge blocking.
+                        # Set to e.g. 100 to ensure minimum 100W charging when mode 8 is active.
+                        # Only affects mode 8; ignored in other modes.


The min_pv_charge_rate comment implies it prevents “complete charge blocking” unless set to 0, but the implementation in Batcontrol.limit_battery_charge_rate() intentionally keeps limit_charge_rate=0 even when min_pv_charge_rate > 0. Update the comment to reflect the real behavior (e.g. min applies only when the requested limit is > 0).

Suggested change

min_pv_charge_rate: 0 # Watt, STATIC lower limit for MODE_LIMIT_BATTERY_CHARGE_RATE (8).

# Set to 0 to allow complete charge blocking.

# Set to e.g. 100 to ensure minimum 100W charging when mode 8 is active.

# Only affects mode 8; ignored in other modes.

min_pv_charge_rate: 0 # Watt, STATIC lower limit for MODE_LIMIT_BATTERY_CHARGE_RATE (8) when a positive limit is requested.

# Set to 0 to disable any enforced minimum (explicit 0 W limit still allows complete charge blocking).

# Set to e.g. 100 to ensure the effective charge limit is at least 100 W when mode 8 requests > 0 W.

# Does not override an explicit limit of 0 W and is ignored in other modes.

src/batcontrol/core.py

set_mode_avoid_discharge was documented as 'allow discharge mode' and set_mode_allow_discharge was documented as 'avoid discharge mode'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The previous comment implied that setting min_pv_charge_rate to 0 was needed to allow complete charge blocking. In reality, the implementation skips the minimum floor when limit_charge_rate == 0, so a requested limit of 0 always results in complete charge blocking regardless of min_pv_charge_rate. The comment now reflects this actual behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

Copilot · 2026-03-10T15:34:37Z

tests/batcontrol/test_core.py

+import sys
+import os
+from unittest.mock import MagicMock, patch
+
+# Add the src directory to Python path for testing
+sys.path.insert(0, os.path.join(
+    os.path.dirname(__file__), '..', '..', 'src'))
+


This test file manually inserts src/ into sys.path, but the repository already does this globally in tests/conftest.py. Keeping this extra path manipulation is redundant and can make import resolution/order differ from the rest of the test suite. Please remove the sys.path.insert(...) block (and the now-unused sys/os imports).

Suggested change

import sys

import os

from unittest.mock import MagicMock, patch

# Add the src directory to Python path for testing

sys.path.insert(0, os.path.join(

os.path.dirname(__file__), '..', '..', 'src'))

from unittest.mock import MagicMock, patch

Copilot · 2026-03-10T15:34:37Z

tests/batcontrol/test_production_offset.py

                # Check that production was offset correctly
                # Note: production[0] is adjusted for elapsed time in current interval
                # so we only check indices [1] and [2] for exact values
-                assert batcontrol.last_production is not None
-                # Check that offset was applied (values should be roughly half)
-                assert batcontrol.last_production[1] == pytest.approx(1000, rel=0.01)
-                assert batcontrol.last_production[2] == pytest.approx(1500, rel=0.01)
-                # For [0], just check it's less than original
-                assert batcontrol.last_production[0] < 500  # Should be ~500 or less due to elapsed time
-
-    def test_production_offset_api_set_valid(self, mock_config):
-        """Test setting production offset via API with valid value"""
-        with patch('batcontrol.core.tariff_factory'), \
-             patch('batcontrol.core.inverter_factory'), \
-             patch('batcontrol.core.solar_factory'), \
-             patch('batcontrol.core.consumption_factory'):
-
-            batcontrol = Batcontrol(mock_config)
-
-            # Set via API
-            batcontrol.api_set_production_offset(0.7)
-
-            # Should be updated
-            assert batcontrol.production_offset_percent == 0.7
-
-    def test_production_offset_api_set_invalid_negative(self, mock_config):
-        """Test setting production offset via API with invalid negative value"""
-        with patch('batcontrol.core.tariff_factory'), \
-             patch('batcontrol.core.inverter_factory'), \
-             patch('batcontrol.core.solar_factory'), \
-             patch('batcontrol.core.consumption_factory'):
-
-            batcontrol = Batcontrol(mock_config)
-            original_value = batcontrol.production_offset_percent
-
-            # Try to set invalid value
-            batcontrol.api_set_production_offset(-0.5)
-
-            # Should not be updated
-            assert batcontrol.production_offset_percent == original_value
+                assert batcontrol.last_production[1] == pytest.approx(1000.0)
+                assert batcontrol.last_production[2] == pytest.approx(1500.0)

    def test_production_offset_api_set_invalid_too_high(self, mock_config):


The tests for api_set_production_offset with a valid value and with a negative invalid value were removed here, but the corresponding API method still exists (Batcontrol.api_set_production_offset). This reduces coverage for production-offset behavior and seems unrelated to the new charge-rate limiting mode. Please restore equivalent tests (or add new ones) to keep both the "valid update" and "invalid (<0) ignored" cases covered.

Copilot · 2026-03-10T15:34:37Z

src/batcontrol/core.py

+        effective_limit = limit_charge_rate
+        if self.max_pv_charge_rate > 0:
+            effective_limit = min(effective_limit, self.max_pv_charge_rate)
+        if self.min_pv_charge_rate > 0 and limit_charge_rate > 0:
+            effective_limit = max(effective_limit, self.min_pv_charge_rate)
+
+        logger.info('Mode: Limit Battery Charge Rate to %d W, discharge allowed', effective_limit)


In limit_battery_charge_rate, the bound application can produce an effective_limit that exceeds max_pv_charge_rate when min_pv_charge_rate is configured higher than max_pv_charge_rate (cap happens before floor). This can result in sending an out-of-range charge limit to the inverter. Please adjust the bounding logic to guarantee effective_limit stays within [0, max_pv_charge_rate] (when max>0), and consider validating/logging when min_pv_charge_rate > max_pv_charge_rate.

Suggested change

effective_limit = limit_charge_rate

if self.max_pv_charge_rate > 0:

effective_limit = min(effective_limit, self.max_pv_charge_rate)

if self.min_pv_charge_rate > 0 and limit_charge_rate > 0:

effective_limit = max(effective_limit, self.min_pv_charge_rate)

logger.info('Mode: Limit Battery Charge Rate to %d W, discharge allowed', effective_limit)

# Always enforce a non-negative limit

effective_limit = max(0, limit_charge_rate)

if self.max_pv_charge_rate > 0:

# Guard against misconfigured min/max values

if (

self.min_pv_charge_rate > 0

and self.min_pv_charge_rate > self.max_pv_charge_rate

):

logger.warning(

"Configured min_pv_charge_rate (%d W) is greater than "

"max_pv_charge_rate (%d W). Adjusting minimum to max.",

self.min_pv_charge_rate,

self.max_pv_charge_rate,

)

min_bound = self.max_pv_charge_rate

else:

min_bound = self.min_pv_charge_rate

# First cap to the configured maximum

effective_limit = min(effective_limit, self.max_pv_charge_rate)

# Then enforce the (possibly adjusted) minimum, without exceeding max

if min_bound > 0 and effective_limit > 0:

effective_limit = max(effective_limit, min_bound)

else:

# No max configured (<= 0): only enforce minimum if both are positive

if self.min_pv_charge_rate > 0 and effective_limit > 0:

effective_limit = max(effective_limit, self.min_pv_charge_rate)

logger.info(

'Mode: Limit Battery Charge Rate to %d W, discharge allowed',

effective_limit,

)

Copilot · 2026-03-10T15:34:38Z

src/batcontrol/mqtt_api.py

+                limit,
+                retain=True


publish_limit_battery_charge_rate publishes with retain=True, but other comparable state topics like /mode, /charge_rate, and most config values are published without retain. This inconsistency can leave a stale retained limit after batcontrol restarts or mode changes. Consider removing retain=True (or consistently retaining all state/config topics if that’s the intended contract).

Suggested change

limit,

retain=True

limit

MaStr · 2026-03-10T15:36:33Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2026-03-10T15:36:41Z

@MaStr I've opened a new pull request, #292, to work on those changes. Once the pull request is ready, I'll request review from you.

… bounds logic, remove MQTT retain (#292) * Initial plan * fix: Address PR review feedback - clean up imports, restore tests, fix bounds logic, remove retain Co-authored-by: MaStr <1036501+MaStr@users.noreply.github.com> * refactor: Move min/max PV charge rate guard to initialization Co-authored-by: MaStr <1036501+MaStr@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: MaStr <1036501+MaStr@users.noreply.github.com>

MaStr requested a review from Copilot February 5, 2026 19:12

Copilot started reviewing on behalf of MaStr February 5, 2026 19:12 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

MaStr self-assigned this Feb 5, 2026

MaStr added this to the 0.7.0 milestone Feb 5, 2026

MaStr mentioned this pull request Feb 20, 2026

Peak Shaving Strategies: Options for Feed-in Management and Battery Overshoot Prevention #271

Open

MaStr and others added 3 commits March 10, 2026 16:00

Enhance reslient wrapper

72f42d5

MaStr force-pushed the max_charge_rate branch from 0ac5188 to 11fe19b Compare March 10, 2026 15:06

MaStr requested a review from Copilot March 10, 2026 15:08

Copilot started reviewing on behalf of MaStr March 10, 2026 15:08 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

MaStr and others added 2 commits March 10, 2026 16:25

fix: Correct swapped docstrings in InverterInterface

00463e9

set_mode_avoid_discharge was documented as 'allow discharge mode' and set_mode_allow_discharge was documented as 'avoid discharge mode'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings March 10, 2026 15:27

Copilot started reviewing on behalf of MaStr March 10, 2026 15:28 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Copilot AI mentioned this pull request Mar 10, 2026

Fix review feedback: clean up imports, restore tests, fix charge rate bounds logic, remove MQTT retain #292

Merged

6 tasks

MaStr merged commit 9d49273 into main Mar 10, 2026
13 checks passed

MaStr deleted the max_charge_rate branch March 10, 2026 16:00

	from unittest.mock import Mock, MagicMock, patch
	from unittest.mock import MagicMock, patch

-        effective_limit = limit_charge_rate
-        if self.max_pv_charge_rate > 0:
-            effective_limit = min(effective_limit, self.max_pv_charge_rate)
-        if self.min_pv_charge_rate > 0 and limit_charge_rate > 0:
-            effective_limit = max(effective_limit, self.min_pv_charge_rate)
-        logger.info('Mode: Limit Battery Charge Rate to %d W, discharge allowed', effective_limit)
+        # Always enforce a non-negative limit
+        effective_limit = max(0, limit_charge_rate)
+        if self.max_pv_charge_rate > 0:
+            # Guard against misconfigured min/max values
+            if (
+                self.min_pv_charge_rate > 0
+                and self.min_pv_charge_rate > self.max_pv_charge_rate
+            ):
+                logger.warning(
+                    "Configured min_pv_charge_rate (%d W) is greater than "
+                    "max_pv_charge_rate (%d W). Adjusting minimum to max.",
+                    self.min_pv_charge_rate,
+                    self.max_pv_charge_rate,
+                )
+                min_bound = self.max_pv_charge_rate
+            else:
+                min_bound = self.min_pv_charge_rate
+            # First cap to the configured maximum
+            effective_limit = min(effective_limit, self.max_pv_charge_rate)
+            # Then enforce the (possibly adjusted) minimum, without exceeding max
+            if min_bound > 0 and effective_limit > 0:
+                effective_limit = max(effective_limit, min_bound)
+        else:
+            # No max configured (<= 0): only enforce minimum if both are positive
+            if self.min_pv_charge_rate > 0 and effective_limit > 0:
+                effective_limit = max(effective_limit, self.min_pv_charge_rate)
+        logger.info(
+            'Mode: Limit Battery Charge Rate to %d W, discharge allowed',
+            effective_limit,
+        )

Conversation

MaStr commented Feb 5, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

MaStr commented Mar 10, 2026

Uh oh!

Copilot AI commented Mar 10, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants