Add BatteryStatus tests to ensure it recovers automatically after issues (#522)

This PR also does:
 1. Improve usability of MockMicrogrid
 2. Update PowerDistributor tests to use MockMicrogrid
 3. And some small bug fixes and code quality improvements

Author: shsms

src/frequenz/sdk/actor/power_distributing/_battery_status.py

@@ -81,6 +81,10 @@ class _BlockingStatus:
     max_duration_sec: float
 
     def __post_init__(self) -> None:
+        assert self.min_duration_sec <= self.max_duration_sec, (
+            f"Minimum blocking duration ({self.min_duration_sec}) cannot be greater "
+            f"than maximum blocking duration ({self.max_duration_sec})"
+        )
         self.last_blocking_duration_sec: float = self.min_duration_sec
         self.blocked_until: Optional[datetime] = None
 
@@ -335,9 +339,25 @@ async def _run(
                         self._handle_status_set_power_result(selected.value)
 
                     elif selected_from(selected, battery_timer):
+                        if (
+                            datetime.now(tz=timezone.utc)
+                            - self._battery.last_msg_timestamp
+                        ) < timedelta(seconds=self._max_data_age):
+                            # This means that we have received data from the battery
+                            # since the timer triggered, but the timer event arrived
+                            # late, so we can ignore it.
+                            continue
                         self._handle_status_battery_timer()
 
                     elif selected_from(selected, inverter_timer):
+                        if (
+                            datetime.now(tz=timezone.utc)
+                            - self._inverter.last_msg_timestamp
+                        ) < timedelta(seconds=self._max_data_age):
+                            # This means that we have received data from the inverter
+                            # since the timer triggered, but the timer event arrived
+                            # late, so we can ignore it.
+                            continue
                         self._handle_status_inverter_timer()
 
                     else:

tests/actor/test_battery_pool_status.py

@@ -5,7 +5,6 @@
 import asyncio
 from typing import Set
 
-import pytest
 from frequenz.channels import Broadcast
 from pytest_mock import MockerFixture
 
@@ -14,41 +13,30 @@
     BatteryStatus,
 )
 from frequenz.sdk.microgrid.component import ComponentCategory
+from tests.timeseries.mock_microgrid import MockMicrogrid
 
-from ..utils.mock_microgrid_client import MockMicrogridClient
-from .test_battery_status import battery_data, component_graph, inverter_data
+from .test_battery_status import battery_data, inverter_data
 
 
 # pylint: disable=protected-access
 class TestBatteryPoolStatus:
     """Tests for BatteryPoolStatus"""
 
-    @pytest.fixture
-    async def mock_microgrid(self, mocker: MockerFixture) -> MockMicrogridClient:
-        """Create and initialize mock microgrid
-
-        Args:
-            mocker: pytest mocker
-
-        Returns:
-            MockMicrogridClient
-        """
-        components, connections = component_graph()
-        microgrid = MockMicrogridClient(components, connections)
-        microgrid.initialize(mocker)
-        return microgrid
-
-    async def test_batteries_status(self, mock_microgrid: MockMicrogridClient) -> None:
+    async def test_batteries_status(self, mocker: MockerFixture) -> None:
         """Basic tests for BatteryPoolStatus.
 
         BatteryStatusTracker is more tested in its own unit tests.
 
         Args:
             mock_microgrid: mock microgrid client
         """
+        mock_microgrid = MockMicrogrid(grid_side_meter=True)
+        mock_microgrid.add_batteries(3)
+        await mock_microgrid.start(mocker)
+
         batteries = {
             battery.component_id
-            for battery in mock_microgrid.component_graph.components(
+            for battery in mock_microgrid.mock_client.component_graph.components(
                 component_category={ComponentCategory.BATTERY}
             )
         }
@@ -67,22 +55,34 @@ async def test_batteries_status(self, mock_microgrid: MockMicrogridClient) -> No
 
         batteries_list = list(batteries)
 
-        await mock_microgrid.send(battery_data(component_id=batteries_list[0]))
+        await mock_microgrid.mock_client.send(
+            battery_data(component_id=batteries_list[0])
+        )
         await asyncio.sleep(0.1)
         assert batteries_status.get_working_batteries(batteries) == expected_working
 
         expected_working.add(batteries_list[0])
-        await mock_microgrid.send(inverter_data(component_id=batteries_list[0] - 1))
+        await mock_microgrid.mock_client.send(
+            inverter_data(component_id=batteries_list[0] - 1)
+        )
         await asyncio.sleep(0.1)
         assert batteries_status.get_working_batteries(batteries) == expected_working
         msg = await asyncio.wait_for(battery_status_recv.receive(), timeout=0.2)
         assert msg == batteries_status._current_status
 
-        await mock_microgrid.send(inverter_data(component_id=batteries_list[1] - 1))
-        await mock_microgrid.send(battery_data(component_id=batteries_list[1]))
+        await mock_microgrid.mock_client.send(
+            inverter_data(component_id=batteries_list[1] - 1)
+        )
+        await mock_microgrid.mock_client.send(
+            battery_data(component_id=batteries_list[1])
+        )
 
-        await mock_microgrid.send(inverter_data(component_id=batteries_list[2] - 1))
-        await mock_microgrid.send(battery_data(component_id=batteries_list[2]))
+        await mock_microgrid.mock_client.send(
+            inverter_data(component_id=batteries_list[2] - 1)
+        )
+        await mock_microgrid.mock_client.send(
+            battery_data(component_id=batteries_list[2])
+        )
 
         expected_working = set(batteries_list)
         await asyncio.sleep(0.1)
@@ -91,15 +91,15 @@ async def test_batteries_status(self, mock_microgrid: MockMicrogridClient) -> No
         assert msg == batteries_status._current_status
 
         await batteries_status.update_status(
-            succeed_batteries={106}, failed_batteries={206, 306}
+            succeed_batteries={9}, failed_batteries={19, 29}
         )
         await asyncio.sleep(0.1)
-        assert batteries_status.get_working_batteries(batteries) == {106}
+        assert batteries_status.get_working_batteries(batteries) == {9}
 
         await batteries_status.update_status(
-            succeed_batteries={106, 206}, failed_batteries=set()
+            succeed_batteries={9, 19}, failed_batteries=set()
         )
         await asyncio.sleep(0.1)
-        assert batteries_status.get_working_batteries(batteries) == {106, 206}
+        assert batteries_status.get_working_batteries(batteries) == {9, 19}
 
         await batteries_status.stop()