Merge V1.0.0 rc6xx into v1.x.x

RELEASE_NOTES.md

@@ -2,4 +2,6 @@
 
 ## Bug Fixes
 
-- Fix getting reactive power from meters, inverters and EV chargers.
+- Fixes a bug in the ring buffer in case the updated value is missing and creates a gap in time.
+
+- Fixed a bug that was causing the `PowerDistributor` to exit if power requests to PV inverters or EV chargers timeout.

src/frequenz/sdk/actor/power_distributing/_component_managers/_ev_charger_manager/_ev_charger_manager.py

@@ -323,29 +323,29 @@ async def _set_api_power(
         succeeded_components: set[int] = set()
         failed_power = Power.zero()
         for component_id, task in tasks.items():
-            exc = task.exception()
-            if exc is not None:
-                failed_components.add(component_id)
-                failed_power += target_power_changes[component_id]
+            try:
+                task.result()
+            except asyncio.CancelledError:
+                _logger.warning(
+                    "Timeout while setting power to EV charger %s", component_id
+                )
+            except grpc.aio.AioRpcError as exc:
+                _logger.warning(
+                    "Error while setting power to EV charger %s: %s",
+                    component_id,
+                    exc,
+                )
+            except Exception:  # pylint: disable=broad-except
+                _logger.exception(
+                    "Unknown error while setting power to EV charger: %s", component_id
+                )
             else:
                 succeeded_components.add(component_id)
+                continue
+
+            failed_components.add(component_id)
+            failed_power += target_power_changes[component_id]
 
-            match task.exception():
-                case asyncio.CancelledError():
-                    _logger.warning(
-                        "Timeout while setting power to EV charger %s", component_id
-                    )
-                case grpc.aio.AioRpcError() as err:
-                    _logger.warning(
-                        "Error while setting power to EV charger %s: %s",
-                        component_id,
-                        err,
-                    )
-                case Exception():
-                    _logger.exception(
-                        "Unknown error while setting power to EV charger: %s",
-                        component_id,
-                    )
         if failed_components:
             return PartialFailure(
                 failed_components=failed_components,

src/frequenz/sdk/actor/power_distributing/_component_managers/_pv_inverter_manager/_pv_inverter_manager.py

@@ -113,11 +113,19 @@ async def distribute_power(self, request: Request) -> None:
             raise ValueError(
                 "Cannot distribute power to PV inverters without any inverters"
             )
-        working_components = list(
-            self._component_pool_status_tracker.get_working_components(
-                request.component_ids
-            )
-        )
+
+        working_components: list[int] = []
+        for inv_id in self._component_pool_status_tracker.get_working_components(
+            request.component_ids
+        ):
+            if self._component_data_caches[inv_id].has_value():
+                working_components.append(inv_id)
+            else:
+                _logger.warning(
+                    "Exclude inverter %s from distribution, because it didn't "
+                    "send any data since the application startup.",
+                    inv_id,
+                )
 
         # When sorting by lower bounds, which are negative for PV inverters, we have to
         # reverse the order, so that the inverters with the higher bounds i.e., the
@@ -130,13 +138,18 @@ async def distribute_power(self, request: Request) -> None:
         )
 
         num_components = len(working_components)
+        if num_components == 0:
+            _logger.error("No inverters available for power distribution. Aborting.")
+            return
+
         for idx, inv_id in enumerate(working_components):
             # Request powers are negative for PV inverters.  When remaining power is
             # greater than 0.0, we can stop allocating further.
             if remaining_power > Power.zero() or is_close_to_zero(
                 remaining_power.as_watts()
             ):
-                break
+                allocations[inv_id] = Power.zero()
+                continue
             distribution = remaining_power / float(num_components - idx)
             inv_data = self._component_data_caches[inv_id]
             if not inv_data.has_value():
@@ -183,29 +196,30 @@ async def _set_api_power(  # pylint: disable=too-many-locals
         succeeded_components: set[int] = set()
         failed_power = Power.zero()
         for component_id, task in tasks.items():
-            exc = task.exception()
-            if exc is not None:
-                failed_components.add(component_id)
-                failed_power += allocations[component_id]
+            try:
+                task.result()
+            except asyncio.CancelledError:
+                _logger.warning(
+                    "Timeout while setting power to PV inverter %s", component_id
+                )
+            except grpc.aio.AioRpcError as exc:
+                _logger.warning(
+                    "Error while setting power to PV inverter %s: %s",
+                    component_id,
+                    exc,
+                )
+            except Exception:  # pylint: disable=broad-except
+                _logger.exception(
+                    "Unknown error while setting power to PV inverter: %s",
+                    component_id,
+                )
             else:
                 succeeded_components.add(component_id)
+                continue
+
+            failed_components.add(component_id)
+            failed_power += allocations[component_id]
 
-            match task.exception():
-                case asyncio.CancelledError():
-                    _logger.warning(
-                        "Timeout while setting power to PV inverter %s", component_id
-                    )
-                case grpc.aio.AioRpcError() as err:
-                    _logger.warning(
-                        "Error while setting power to PV inverter %s: %s",
-                        component_id,
-                        err,
-                    )
-                case Exception():
-                    _logger.exception(
-                        "Unknown error while setting power to PV inverter: %s",
-                        component_id,
-                    )
         if failed_components:
             await self._results_sender.send(
                 PartialFailure(

src/frequenz/sdk/timeseries/_ringbuffer/buffer.py

@@ -491,8 +491,11 @@ def _update_gaps(
         # New missing entry that is not already in a gap?
         if record_as_missing:
             if not found_in_gaps:
+                # If there are no gaps and the new value is not subsequent to the
+                # newest value, we need to start the new gap after the newest value
+                start_gap = min(newest + self._sampling_period, timestamp)
                 self._gaps.append(
-                    Gap(start=timestamp, end=timestamp + self._sampling_period)
+                    Gap(start=start_gap, end=timestamp + self._sampling_period)
                 )
         elif len(self._gaps) > 0:
             if found_in_gaps:

tests/timeseries/_pv_pool/test_pv_pool_control_methods.py

@@ -131,7 +131,7 @@ async def _recv_reports_until(
             if check(report):
                 break
 
-    async def test_setting_power(
+    async def test_setting_power(  # pylint: disable=too-many-statements
         self,
         mocks: _Mocks,
         mocker: MockerFixture,
@@ -260,3 +260,24 @@ async def test_setting_power(
             mocker.call(inv_ids[2], -30000.0),
             mocker.call(inv_ids[3], -30000.0),
         ]
+
+        # Setting 0 power should set all inverters to 0
+        set_power.reset_mock()
+        await pv_pool.propose_power(Power.zero())
+        await self._recv_reports_until(
+            bounds_rx,
+            lambda x: x.target_power is not None and x.target_power.as_watts() == 0.0,
+        )
+        self._assert_report(
+            await bounds_rx.receive(), power=0.0, lower=-100000.0, upper=0.0
+        )
+        await asyncio.sleep(0.0)
+
+        assert set_power.call_count == 4
+        inv_ids = mocks.microgrid.pv_inverter_ids
+        assert sorted(set_power.call_args_list, key=lambda x: x.args[0]) == [
+            mocker.call(inv_ids[0], 0.0),
+            mocker.call(inv_ids[1], 0.0),
+            mocker.call(inv_ids[2], 0.0),
+            mocker.call(inv_ids[3], 0.0),
+        ]

tests/timeseries/test_ringbuffer.py

@@ -277,12 +277,12 @@ def test_gaps() -> None:  # pylint: disable=too-many-statements
     assert buffer.count_covered() == 5
     assert len(buffer.gaps) == 0
 
-    # whole range gap suffers from sdk#646
+    # whole range gap
     buffer.update(Sample(dt(99), None))
-    assert buffer.oldest_timestamp == dt(95)  # bug: should be None
-    assert buffer.newest_timestamp == dt(99)  # bug: should be None
-    assert buffer.count_valid() == 4  # bug: should be 0 (whole range gap)
-    assert buffer.count_covered() == 5  # bug: should be 0
+    assert buffer.oldest_timestamp is None
+    assert buffer.newest_timestamp is None
+    assert buffer.count_valid() == 0
+    assert buffer.count_covered() == 0
     assert len(buffer.gaps) == 1