diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 1b7dcc491..73f8ac0a5 100644 --- a/RELEASE_NOTES.md +++ b/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. diff --git a/src/frequenz/sdk/actor/power_distributing/_component_managers/_ev_charger_manager/_ev_charger_manager.py b/src/frequenz/sdk/actor/power_distributing/_component_managers/_ev_charger_manager/_ev_charger_manager.py index 2ac8b8254..9cfd163ec 100644 --- a/src/frequenz/sdk/actor/power_distributing/_component_managers/_ev_charger_manager/_ev_charger_manager.py +++ b/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, diff --git a/src/frequenz/sdk/actor/power_distributing/_component_managers/_pv_inverter_manager/_pv_inverter_manager.py b/src/frequenz/sdk/actor/power_distributing/_component_managers/_pv_inverter_manager/_pv_inverter_manager.py index 2a2f77852..38876f1e9 100644 --- a/src/frequenz/sdk/actor/power_distributing/_component_managers/_pv_inverter_manager/_pv_inverter_manager.py +++ b/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( diff --git a/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py b/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py index f431ab039..bac39a46d 100644 --- a/src/frequenz/sdk/timeseries/_ringbuffer/buffer.py +++ b/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: diff --git a/tests/timeseries/_pv_pool/test_pv_pool_control_methods.py b/tests/timeseries/_pv_pool/test_pv_pool_control_methods.py index 492a01ead..1a6cea279 100644 --- a/tests/timeseries/_pv_pool/test_pv_pool_control_methods.py +++ b/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), + ] diff --git a/tests/timeseries/test_ringbuffer.py b/tests/timeseries/test_ringbuffer.py index b3da1292d..ffdb5cd96 100644 --- a/tests/timeseries/test_ringbuffer.py +++ b/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