Move always_allow_discharge_limit float conversion to core.py (Batcontrol main)

Co-authored-by: MaStr <1036501+MaStr@users.noreply.github.com>

Author: Copilot

src/batcontrol/core.py

@@ -38,6 +38,23 @@
 DELAY_EVALUATION_BY_SECONDS = 15  # Delay evaluation for x seconds at every trigger
 # Interval between evaluations in seconds
 TIME_BETWEEN_EVALUATIONS = EVALUATIONS_EVERY_MINUTES * 60
+
+
+def _to_float(value) -> float:
+    """ Convert a config or API value to float, handling European comma decimal notation.
+    Args:
+        value: The value to convert. Can be a float, int, or string
+               (including European notation like '0,9').
+    Returns:
+        float: The converted value.
+    Raises:
+        ValueError: If the value cannot be converted to float.
+    """
+    if isinstance(value, str):
+        return float(value.replace(',', '.'))
+    return float(value)
+
+
 TIME_BETWEEN_UTILITY_API_CALLS = 900  # 15 Minutes
 MIN_FORECAST_HOURS = 1  # Minimum required forecast hours
 FORECAST_TOLERANCE = 3  # Acceptable tolerance for forecast hours
@@ -60,7 +77,8 @@ def __init__(self, configdict: dict):
         # -1 = charge from grid , 0 = avoid discharge , 8 = limit battery charge, 10 = discharge allowed
         self.last_mode = None
         self.last_charge_rate = 0
-        self._limit_battery_charge_rate = -1  # Dynamic battery charge rate limit (-1 = no limit)
+        # Dynamic battery charge rate limit (-1 = no limit)
+        self._limit_battery_charge_rate = -1
         self.last_prices = None
         self.last_consumption = None
         self.last_production = None
@@ -155,7 +173,8 @@ def __init__(self, configdict: dict):
             '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)
+        self.min_pv_charge_rate = config['inverter'].get(
+            'min_pv_charge_rate', 0)
 
         # Validate min/max PV charge rate configuration at startup
         if (
@@ -214,8 +233,8 @@ def __init__(self, configdict: dict):
         self.general_logic = CommonLogic.get_instance(
             charge_rate_multiplier=self.batconfig.get(
                 'charge_rate_multiplier', 1.1),
-            always_allow_discharge_limit=self.batconfig.get(
-                'always_allow_discharge_limit', 0.9),
+            always_allow_discharge_limit=_to_float(self.batconfig.get(
+                'always_allow_discharge_limit', 0.9)),
             max_capacity=self.inverter.get_max_capacity(),
             min_charge_energy=self.batconfig.get('min_recharge_amount', 100.0)
         )
@@ -363,7 +382,8 @@ def handle_forecast_error(self):
 
     def run(self):
         """ Main calculation & control loop """
-        logger.debug('Timeslots are in %d-minute intervals', self.time_resolution)
+        logger.debug('Timeslots are in %d-minute intervals',
+                     self.time_resolution)
 
         # Reset some values
         self.__reset_run_data()
@@ -542,7 +562,8 @@ def run(self):
 
         if inverter_settings.allow_discharge:
             if inverter_settings.limit_battery_charge_rate >= 0:
-                self.limit_battery_charge_rate(inverter_settings.limit_battery_charge_rate)
+                self.limit_battery_charge_rate(
+                    inverter_settings.limit_battery_charge_rate)
             else:
                 self.allow_discharging()
         elif inverter_settings.charge_from_grid:
@@ -611,7 +632,8 @@ def limit_battery_charge_rate(self, limit_charge_rate: int = 0):
             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)
+        logger.info(
+            'Mode: Limit Battery Charge Rate to %d W, discharge allowed', effective_limit)
         self.inverter.set_mode_limit_battery_charge(effective_limit)
         self.__set_mode(MODE_LIMIT_BATTERY_CHARGE_RATE)
 
@@ -723,11 +745,10 @@ def set_discharge_limit(self, discharge_limit) -> None:
     def set_always_allow_discharge_limit(
             self, always_allow_discharge_limit: float) -> None:
         """ Set the always allow discharge limit for battery control """
-        self.general_logic.set_always_allow_discharge_limit(
-            always_allow_discharge_limit)
+        limit = _to_float(always_allow_discharge_limit)
+        self.general_logic.set_always_allow_discharge_limit(limit)
         if self.mqtt_api is not None:
-            self.mqtt_api.publish_always_allow_discharge_limit(
-                always_allow_discharge_limit)
+            self.mqtt_api.publish_always_allow_discharge_limit(limit)
 
     def get_always_allow_discharge_limit(self) -> float:
         """ Get the always allow discharge limit for battery control """
@@ -848,7 +869,8 @@ def api_set_limit_battery_charge_rate(self, limit: int):
             limit: Maximum battery charge rate in W (0 = no charging, -1 = no limit)
         """
         if limit < -1:
-            logger.warning('API: Invalid limit_battery_charge_rate %d W', limit)
+            logger.warning(
+                'API: Invalid limit_battery_charge_rate %d W', limit)
             return
 
         logger.info('API: Setting limit_battery_charge_rate to %d W', limit)

src/batcontrol/logic/common.py

@@ -16,17 +16,14 @@
 logger = logging.getLogger(__name__)
 
 # Singleton pattern to ensure only one instance of CommonLogic exists
-
-
 class CommonLogic:
     """ General logic for battery control that is not specific to control strategies. """
 
     _instance = None  # Singleton instance
     charge_rate_multiplier: float
     always_allow_discharge_limit: float
     max_capacity: float  # Maximum capacity of the battery in Wh
-    # Minimum amount of energy before charging from grid in Wh
-    min_charge_energy: float = 100
+    min_charge_energy: float = 100  # Minimum amount of energy before charging from grid in Wh
 
     @classmethod
     def get_instance(cls, charge_rate_multiplier=1.1,
@@ -37,9 +34,9 @@ def get_instance(cls, charge_rate_multiplier=1.1,
         if cls._instance is None:
             cls._instance = cls.__new__(cls)
             cls._instance.initialize(charge_rate_multiplier,
-                                     always_allow_discharge_limit,
-                                     max_capacity,
-                                     min_charge_energy)
+                                 always_allow_discharge_limit,
+                                 max_capacity,
+                                 min_charge_energy)
         return cls._instance
 
     def __init__(self, *args, **kwargs):
@@ -50,41 +47,25 @@ def __init__(self, *args, **kwargs):
         self.initialize(*args, **kwargs)
 
     def initialize(self, charge_rate_multiplier=1.1,
-                   always_allow_discharge_limit=0.9,
-                   max_capacity=10000,
-                   min_charge_energy=100):
+               always_allow_discharge_limit=0.9,
+               max_capacity=10000,
+               min_charge_energy=100):
         """ Private initialization method. """
-        self.charge_rate_multiplier = float(charge_rate_multiplier)
-        self.always_allow_discharge_limit = self._to_float(
-            always_allow_discharge_limit)
-        self.max_capacity = float(max_capacity)
-        self.min_charge_energy = float(min_charge_energy)
+        self.charge_rate_multiplier = charge_rate_multiplier
+        self.always_allow_discharge_limit = always_allow_discharge_limit
+        self.max_capacity = max_capacity
+        self.min_charge_energy = min_charge_energy
 
     def set_charge_rate_multiplier(self, multiplier: float):
         """ Set the charge rate multiplier. """
         logger.debug('Setting charge rate multiplier to %s', multiplier)
         self.charge_rate_multiplier = multiplier
 
-    @staticmethod
-    def _to_float(value) -> float:
-        """ Convert a value to float, handling European comma decimal notation.
-        Args:
-            value: The value to convert. Can be a float, int, or string
-                   (including European notation like '0,9').
-        Returns:
-            float: The converted value.
-        Raises:
-            ValueError: If the value cannot be converted to float.
-        """
-        if isinstance(value, str):
-            return float(value.replace(',', '.'))
-        return float(value)
-
     def set_always_allow_discharge_limit(self, limit: float):
         """ Set the always allowed discharge limit. """
         logger.debug(
             'Setting always allowed discharge limit to %s', limit)
-        self.always_allow_discharge_limit = self._to_float(limit)
+        self.always_allow_discharge_limit = limit
 
     def get_always_allow_discharge_limit(self) -> float:
         """ Get the always allowed discharge limit. """
@@ -111,11 +92,11 @@ def is_discharge_always_allowed_capacity(self, capacity: float) -> bool:
 
         if capacity >= self.max_capacity * self.always_allow_discharge_limit:
             logger.debug(
-                'Discharge is \'always allowed\' for current capacity: %.0f Wh', round(capacity, 0))
+                'Discharge is \'always allowed\' for current capacity: %.0f Wh', round(capacity,0))
             return True
 
         logger.debug(
-            'Discharge is NOT \'always allowed\' for current capacity: %.0f Wh', round(capacity, 0))
+            'Discharge is NOT \'always allowed\' for current capacity: %.0f Wh', round(capacity,0))
         return False
 
     def is_charging_above_minimum(self, needed_energy: float) -> bool:

tests/batcontrol/logic/test_common.py

@@ -43,25 +43,6 @@ def test_set_always_allowed_discharge_limit(self):
         self.assertEqual(self.logic.always_allow_discharge_limit, new_limit)
         self.assertEqual(self.logic.get_always_allow_discharge_limit(), new_limit)
 
-    def test_set_always_allowed_discharge_limit_string(self):
-        """Test setting the always allowed discharge limit with a string value (dot notation)"""
-        self.logic.set_always_allow_discharge_limit('0.85')
-        self.assertIsInstance(self.logic.always_allow_discharge_limit, float)
-        self.assertAlmostEqual(self.logic.always_allow_discharge_limit, 0.85)
-
-    def test_set_always_allowed_discharge_limit_european_comma(self):
-        """Test setting the always allowed discharge limit with European comma notation"""
-        self.logic.set_always_allow_discharge_limit('0,85')
-        self.assertIsInstance(self.logic.always_allow_discharge_limit, float)
-        self.assertAlmostEqual(self.logic.always_allow_discharge_limit, 0.85)
-
-    def test_initialize_always_allow_discharge_limit_from_string(self):
-        """Test that initialize converts always_allow_discharge_limit string to float"""
-        CommonLogic._instance = None
-        logic = CommonLogic.get_instance(always_allow_discharge_limit='0,75')
-        self.assertIsInstance(logic.always_allow_discharge_limit, float)
-        self.assertAlmostEqual(logic.always_allow_discharge_limit, 0.75)
-
     def test_is_discharge_always_allowed_soc(self):
         """Test discharge always allowed when SOC is above threshold"""
         # SOC above the threshold (90%)

tests/batcontrol/test_core.py

@@ -287,5 +287,84 @@ def test_api_set_limit_applies_immediately_in_mode_8(
         mock_inverter.set_mode_limit_battery_charge.assert_called_once_with(2000)
 
 
+class TestAlwaysAllowDischargeLimitConversion:
+    """Tests that always_allow_discharge_limit is always a float in core.py"""
+
+    @pytest.fixture
+    def mock_inverter(self):
+        """Create a mock inverter"""
+        inv = MagicMock()
+        inv.max_pv_charge_rate = 3000
+        inv.set_mode_limit_battery_charge = MagicMock()
+        inv.get_max_capacity = MagicMock(return_value=10000)
+        return inv
+
+    def _make_batcontrol(self, mock_inverter, always_allow_discharge_limit):
+        """Helper to create a Batcontrol instance with a given discharge limit config"""
+        config = {
+            'timezone': 'Europe/Berlin',
+            'time_resolution_minutes': 60,
+            'inverter': {
+                'type': 'dummy',
+                'max_grid_charge_rate': 5000,
+                'max_pv_charge_rate': 3000,
+                'min_pv_charge_rate': 100
+            },
+            'utility': {'type': 'tibber', 'token': 'test_token'},
+            'pvinstallations': [],
+            'consumption_forecast': {'type': 'simple', 'value': 500},
+            'battery_control': {
+                'max_charging_from_grid_limit': 0.5,
+                'min_price_difference': 0.05,
+                'always_allow_discharge_limit': always_allow_discharge_limit,
+            },
+            'mqtt': {'enabled': False}
+        }
+        with patch('batcontrol.core.tariff_factory.create_tarif_provider'), \
+             patch('batcontrol.core.inverter_factory.create_inverter',
+                   return_value=mock_inverter), \
+             patch('batcontrol.core.solar_factory.create_solar_provider'), \
+             patch('batcontrol.core.consumption_factory.create_consumption'):
+            from batcontrol.logic.common import CommonLogic
+            CommonLogic._instance = None
+            return Batcontrol(config)
+
+    def test_config_string_dot_notation(self, mock_inverter):
+        """Config value '0.9' (string) is converted to float 0.9"""
+        bc = self._make_batcontrol(mock_inverter, '0.9')
+        result = bc.get_always_allow_discharge_limit()
+        assert isinstance(result, float)
+        assert abs(result - 0.9) < 1e-9
+
+    def test_config_european_comma_notation(self, mock_inverter):
+        """Config value '0,9' (European decimal) is converted to float 0.9"""
+        bc = self._make_batcontrol(mock_inverter, '0,9')
+        result = bc.get_always_allow_discharge_limit()
+        assert isinstance(result, float)
+        assert abs(result - 0.9) < 1e-9
+
+    def test_setter_string_dot_notation(self, mock_inverter):
+        """Setter with string '0.85' is converted to float"""
+        bc = self._make_batcontrol(mock_inverter, 0.9)
+        bc.set_always_allow_discharge_limit('0.85')
+        result = bc.get_always_allow_discharge_limit()
+        assert isinstance(result, float)
+        assert abs(result - 0.85) < 1e-9
+
+    def test_setter_european_comma_notation(self, mock_inverter):
+        """Setter with European '0,85' is converted to float"""
+        bc = self._make_batcontrol(mock_inverter, 0.9)
+        bc.set_always_allow_discharge_limit('0,85')
+        result = bc.get_always_allow_discharge_limit()
+        assert isinstance(result, float)
+        assert abs(result - 0.85) < 1e-9
+
+    def test_getter_always_returns_float(self, mock_inverter):
+        """get_always_allow_discharge_limit always returns a float"""
+        bc = self._make_batcontrol(mock_inverter, 0.9)
+        result = bc.get_always_allow_discharge_limit()
+        assert isinstance(result, float)
+
+
 if __name__ == '__main__':
     pytest.main([__file__, '-v'])