Repository: frequenz-floss/frequenz-channels-python (language: python)

Issue:
Backport timer `reset()` fix to v0.16
### What happened?

This issue:

* #241

### What did you expect instead?

The timer resetting properly.

### Affected version(s)

v0.15+

### Affected part(s)

Utility receivers, `Event`, `FileWatcher`, `Timer`, etc. (part:receivers)

### Extra information

We need this backported to v0.16 becasue the SDK v0.25 uses it.


Discussion / hints:
FYI @daniel-zullo-frequenz @shsms 

Fix patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index 9006bb22..e5107dc7 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -1,91 +1,5 @@
 # Frequenz Channels Release Notes
 
-## Summary
+## Bug Fixes
 
-The minimum Python supported version was bumped to 3.11 and the `Select` class replaced by the new `select()` function.
-
-## Upgrading
-
-* The minimum supported Python version was bumped to 3.11, downstream projects will need to upgrade too to use this version.
-
-* The `Select` class was replaced by a new `select()` function, with the following improvements:
-
-  * Type-safe: proper type hinting by using the new helper type guard `selected_from()`.
-  * Fixes potential starvation issues.
-  * Simplifies the interface by providing values one-by-one.
-  * Guarantees there are no dangling tasks left behind when used as an async context manager.
-
-  This new function is an [async iterator](https://docs.python.org/3.11/library/collections.abc.html#collections.abc.AsyncIterator), and makes sure no dangling tasks are left behind after a select loop is done.
-
-  Example:
-  ```python
-  timer1 = Timer.periodic(datetime.timedelta(seconds=1))
-  timer2 = Timer.timeout(datetime.timedelta(seconds=0.5))
-
-  async for selected in select(timer1, timer2):
-      if selected_from(selected, timer1):
-          # Beware: `selected.value` might raise an exception, you can always
-          # check for exceptions with `selected.exception` first or use
-          # a try-except block. You can also quickly check if the receiver was
-          # stopped and let any other unexpected exceptions bubble up.
-          if selected.was_stopped():
-              print("timer1 was stopped")
-              continue
-          print(f"timer1: now={datetime.datetime.now()} drift={selected.value}")
-          timer2.stop()
-      elif selected_from(selected, timer2):
-          # Explicitly handling of exceptions
-          match selected.exception:
-              case ReceiverStoppedError():
-                  print("timer2 was stopped")
-              case Exception() as exception:
-                  print(f"timer2: exception={exception}")
-              case None:
-                  # All good, no exception, we can use `selected.value` safely
-                  print(
-                      f"timer2: now={datetime.datetime.now()} "
-                      f"drift={selected.value}"
-                  )
-              case _ as unhanded:
-                  assert_never(unhanded)
-      else:
-          # This is not necessary, as select() will check for exhaustiveness, but
-          # it is good practice to have it in case you forgot to handle a new
-          # receiver added to `select()` at a later point in time.
-          assert False
-  ```
-
-## New Features
-
-* A new `select()` function was added, please look at the *Upgrading* section for details.
-
-* A new `Event` utility receiver was added.
-
-  This receiver can be made ready manually.  It is mainly useful for testing but can also become handy in scenarios where a simple, on-off signal needs to be sent to a select loop for example.
-
-  Example:
-
-  ```python
-  import asyncio
-  from frequenz.channels import Receiver
-  from frequenz.channels.util import Event, select, selected_from
-
-  other_receiver: Receiver[int] = ...
-  exit_event = Event()
-
-  async def exit_after_10_seconds() -> None:
-      asyncio.sleep(10)
-      exit_event.set()
-
-  asyncio.ensure_future(exit_after_10_seconds())
-
-  async for selected in select(exit_event, other_receiver):
-      if selected_from(selected, exit_event):
-          break
-      if selected_from(selected, other_receiver):
-          print(selected.value)
-      else:
-          assert False, "Unknow receiver selected"
-  ```
-
-* The `Timer` class now has more descriptive `__str__` and `__repr__` methods.
+* `Timer`: Fix bug that was causing calls to `reset()` to not reset the timer, if the timer was already being awaited.
diff --git a/src/frequenz/channels/util/_timer.py b/src/frequenz/channels/util/_timer.py
index 02c764d6..45956dd1 100644
--- a/src/frequenz/channels/util/_timer.py
+++ b/src/frequenz/channels/util/_timer.py
@@ -652,10 +652,15 @@ async def ready(self) -> bool:
 
         now = self._now()
         time_to_next_tick = self._next_tick_time - now
+
         # If we didn't reach the tick yet, sleep until we do.
-        if time_to_next_tick > 0:
+        # We need to do this in a loop also reacting to the reset event, as the timer
+        # could be reset while we are sleeping, in which case we need to recalculate
+        # the time to the next tick and try again.
+        while time_to_next_tick > 0:
             await asyncio.sleep(time_to_next_tick / 1_000_000)
             now = self._now()
+            time_to_next_tick = self._next_tick_time - now
 
         # If a stop was explicitly requested during the sleep, we bail out.
         if self._stopped:


Test patch:
diff --git a/tests/utils/test_timer_integration.py b/tests/utils/test_timer_integration.py
new file mode 100644
index 00000000..c9d59049
--- /dev/null
+++ b/tests/utils/test_timer_integration.py
@@ -0,0 +1,34 @@
+# License: MIT
+# Copyright © 2023 Frequenz Energy-as-a-Service GmbH
+
+"""Integration tests for the timer."""
+
+
+import asyncio
+from datetime import timedelta
+
+import async_solipsism
+import pytest
+
+from frequenz.channels.util import Timer
+
+
+@pytest.mark.integration
+async def test_timer_timeout_reset(
+    event_loop: async_solipsism.EventLoop,  # pylint: disable=redefined-outer-name
+) -> None:
+    """Test that the receiving is properly adjusted after a reset."""
+
+    async def timer_wait(timer: Timer) -> None:
+        await timer.receive()
+
+    async with asyncio.timeout(2.0):
+        async with asyncio.TaskGroup() as task_group:
+            timer = Timer.timeout(timedelta(seconds=1.0))
+            start_time = event_loop.time()
+            task_group.create_task(timer_wait(timer))
+            await asyncio.sleep(0.5)
+            timer.reset()
+
+    run_time = event_loop.time() - start_time
+    assert run_time >= 1.5