PowerDistributor should not block user command. (#92)

This is workaround for the bug.
Previously if all components were considered as broken, then all user
commands were blocked for 30 seconds. With this workaround if all
components are considered as broken then we will try to use them anyway.

Author: ela-kotulska-frequenz

src/frequenz/sdk/actor/power_distributing.py

@@ -91,6 +91,34 @@ def update_retry(self, timeout_sec: float) -> None:
         """
         self._timeout_sec = timeout_sec
 
+    def get_working_subset(self, components_ids: Set[int]) -> Set[int]:
+        """Get subset of batteries that are not marked as broken.
+
+        If all given batteries are broken, then mark them as working and return them.
+        This is temporary workaround to not block user command.
+
+        Args:
+            components_ids: set of component ids
+
+        Returns:
+            Subset of given components_ids with working components.
+        """
+        working = set(filter(lambda cid: not self.is_broken(cid), components_ids))
+
+        if len(working) == 0:
+            _logger.warning(
+                "All requested components: %s are marked as broken. "
+                "Marking them as working to not block command.",
+                str(components_ids),
+            )
+
+            for cid in components_ids:
+                self._broken.pop(cid, None)
+
+            working = components_ids
+
+        return working
+
     def is_broken(self, component_id: int) -> bool:
         """Check if component is marked as broken.
 
@@ -595,18 +623,15 @@ def _get_components_data(self, batteries: Iterable[int]) -> List[InvBatPair]:
             Pairs of battery and adjacent inverter data.
         """
         pairs_data: List[InvBatPair] = []
-        for battery_id in batteries:
+
+        for battery_id in self._broken_components.get_working_subset(batteries):
             if battery_id not in self._battery_receivers:
                 raise KeyError(
                     f"No battery {battery_id}, "
                     f"available batteries: {list(self._battery_receivers.keys())}"
                 )
 
             inverter_id: int = self._bat_inv_map[battery_id]
-            if self._broken_components.is_broken(
-                battery_id
-            ) or self._broken_components.is_broken(inverter_id):
-                continue
 
             battery_data: Optional[BatteryData] = self._battery_receivers[
                 battery_id

tests/actor/test_power_distributing.py

@@ -13,11 +13,13 @@
 
 from frequenz.channels import Bidirectional, Broadcast, Receiver, Sender
 from google.protobuf.empty_pb2 import Empty  # pylint: disable=no-name-in-module
+from pytest_mock import MockerFixture
 
 from frequenz.sdk.actor.power_distributing import (
     PowerDistributingActor,
     Request,
     Result,
+    _BrokenComponents,
 )
 from frequenz.sdk.microgrid._graph import _MicrogridComponentGraph
 from frequenz.sdk.microgrid.client import Connection
@@ -738,3 +740,43 @@ async def test_power_distributor_stale_all_components_message(
         assert result.status == Result.Status.ERROR
         # User is interested in batteries only.
         assert result.error_message == "No data for the given batteries {106, 206}"
+
+
+class TestBrokenComponents:
+    """Test if BrokenComponents class is working as expected."""
+
+    def test_broken_components(self, mocker: MockerFixture) -> None:
+        """Check if components are blocked for 30 seconds.
+
+        Args:
+            mocker: pytest mocker
+        """
+        datetime_mock = mocker.patch("frequenz.sdk.actor.power_distributing.datetime")
+
+        expected_datetime = [
+            datetime.fromisoformat("2001-01-01T00:00:00+00:00"),
+            datetime.fromisoformat("2001-01-01T00:00:10+00:00"),
+            datetime.fromisoformat("2001-01-01T00:00:20+00:00"),
+        ]
+        expected_datetime.extend(
+            20 * [datetime.fromisoformat("2001-01-01T00:00:31+00:00")]
+        )
+
+        datetime_mock.now.side_effect = expected_datetime
+
+        # After 30 seconds components should be considered as working
+        broken = _BrokenComponents(30)
+
+        for component_id in range(3):
+            broken.mark_as_broken(component_id)
+
+        # Component 0 was marked as broken 30 seconds ago. Other components, not.
+        assert not broken.is_broken(0)
+        assert broken.is_broken(1)
+        assert broken.is_broken(2)
+        assert broken.get_working_subset({0, 1}) == {0}
+        assert broken.get_working_subset({0}) == {0}
+
+        # If all requested components are marked as broken,
+        # then we should mark them as working to not block user command.
+        assert broken.get_working_subset({1, 2}) == {1, 2}