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

src/batcontrol/core.py

@@ -149,10 +149,28 @@ def __init__(self, configdict: dict):
         self.inverter = inverter_factory.create_inverter(
             config['inverter'])
 
-        # Get PV charge rate limits from inverter config (with defaults)
-        self.max_pv_charge_rate = getattr(self.inverter, 'max_pv_charge_rate', 0)
+        # Get PV charge rate limits from inverter config (with defaults),
+        # falling back to inverter attribute for backward compatibility
+        self.max_pv_charge_rate = config['inverter'].get(
+            'max_pv_charge_rate',
+            getattr(self.inverter, 'max_pv_charge_rate', 0),
+        )
         self.min_pv_charge_rate = config['inverter'].get('min_pv_charge_rate', 0)
 
+        # Validate min/max PV charge rate configuration at startup
+        if (
+            self.max_pv_charge_rate > 0
+            and 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,
+            )
+            self.min_pv_charge_rate = self.max_pv_charge_rate
+
         self.pvsettings = config['pvinstallations']
         self.fc_solar = solar_factory.create_solar_provider(
             self.pvsettings,
@@ -579,12 +597,19 @@ def limit_battery_charge_rate(self, limit_charge_rate: int = 0):
             self.allow_discharging()
             return
 
-        # Apply bounds from config
-        effective_limit = limit_charge_rate
+        # Always enforce a non-negative limit
+        effective_limit = max(0, limit_charge_rate)
+
         if self.max_pv_charge_rate > 0:
+            # Cap to the configured maximum
             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)
+            # Enforce minimum (guaranteed <= max_pv_charge_rate from init validation)
+            if self.min_pv_charge_rate > 0 and effective_limit > 0:
+                effective_limit = max(effective_limit, self.min_pv_charge_rate)
+        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)
         self.inverter.set_mode_limit_battery_charge(effective_limit)

src/batcontrol/mqtt_api.py

@@ -200,8 +200,7 @@ def publish_limit_battery_charge_rate(self, limit: int) -> None:
         if self.client.is_connected():
             self.client.publish(
                 self.base_topic + '/limit_battery_charge_rate',
-                limit,
-                retain=True
+                limit
             )
 
     def publish_production(

tests/batcontrol/test_core.py

@@ -1,19 +1,10 @@
 """Tests for core batcontrol functionality including MODE_LIMIT_BATTERY_CHARGE_RATE"""
 import pytest
-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 batcontrol.core import (
     Batcontrol,
-    MODE_ALLOW_DISCHARGING,
-    MODE_AVOID_DISCHARGING,
     MODE_LIMIT_BATTERY_CHARGE_RATE,
-    MODE_FORCE_CHARGING
 )
 
 
@@ -170,6 +161,39 @@ def test_limit_battery_charge_rate_zero_allowed(
         # Verify it was set to 0 (charging blocked)
         mock_inverter.set_mode_limit_battery_charge.assert_called_once_with(0)
 
+    @patch('batcontrol.core.tariff_factory.create_tarif_provider')
+    @patch('batcontrol.core.inverter_factory.create_inverter')
+    @patch('batcontrol.core.solar_factory.create_solar_provider')
+    @patch('batcontrol.core.consumption_factory.create_consumption')
+    def test_limit_battery_charge_rate_min_exceeds_max(
+        self, mock_consumption, mock_solar, mock_inverter_factory, mock_tariff,
+        mock_config):
+        """Test that when min_pv_charge_rate > max_pv_charge_rate, min is clamped to max at init"""
+        mock_config['inverter']['min_pv_charge_rate'] = 4000
+
+        # Setup mocks
+        mock_inverter = MagicMock()
+        mock_inverter.max_pv_charge_rate = 3000
+        mock_inverter.set_mode_limit_battery_charge = MagicMock()
+        mock_inverter.get_max_capacity = MagicMock(return_value=10000)
+        mock_inverter_factory.return_value = mock_inverter
+
+        mock_tariff.return_value = MagicMock()
+        mock_solar.return_value = MagicMock()
+        mock_consumption.return_value = MagicMock()
+
+        # Create Batcontrol instance — misconfiguration is corrected at init
+        bc = Batcontrol(mock_config)
+
+        # min_pv_charge_rate should have been clamped to max_pv_charge_rate at init
+        assert bc.min_pv_charge_rate == 3000
+
+        # Set any positive limit - should be clamped to max_pv_charge_rate (3000)
+        bc.limit_battery_charge_rate(1000)
+
+        # Verify effective limit does not exceed max_pv_charge_rate
+        mock_inverter.set_mode_limit_battery_charge.assert_called_once_with(3000)
+
     @patch('batcontrol.core.tariff_factory.create_tarif_provider')
     @patch('batcontrol.core.inverter_factory.create_inverter')
     @patch('batcontrol.core.solar_factory.create_solar_provider')

tests/batcontrol/test_production_offset.py

@@ -165,6 +165,37 @@ def test_production_offset_applied_to_forecast(self, mock_config):
                 assert batcontrol.last_production[1] == pytest.approx(1000.0)
                 assert batcontrol.last_production[2] == pytest.approx(1500.0)
 
+    def test_production_offset_api_set_valid(self, mock_config):
+        """Test setting production offset via API with a valid mid-range 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 a typical valid value (70% of production)
+            batcontrol.api_set_production_offset(0.7)
+
+            # Should be updated
+            assert batcontrol.production_offset_percent == pytest.approx(0.7)
+
+    def test_production_offset_api_set_invalid_negative(self, mock_config):
+        """Test setting production offset via API with an 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 negative value
+            batcontrol.api_set_production_offset(-0.5)
+
+            # Should not be updated
+            assert batcontrol.production_offset_percent == original_value
+
     def test_production_offset_api_set_invalid_too_high(self, mock_config):
         """Test setting production offset via API with invalid high value"""
         with patch('batcontrol.core.tariff_factory'), \