Add a wall clock attached timer #1249
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We expose `ResamplingError` and `SourceStoppedError` as users might need to catch these errors. Signed-off-by: Leandro Lucarella <[email protected]>
This timer uses the wall clock to trigger ticks and handles discrepancies between the wall clock and monotonic time. Since sleeping is performed using monotonic time, differences between the two clocks can occur. When the wall clock progresses slower than monotonic time, it is referred to as *compression* (wall clock time appears in the past relative to monotonic time). Conversely, when the wall clock progresses faster, it is called *expansion* (wall clock time appears in the future relative to monotonic time). If these differences exceed a configured threshold, a warning is emitted. If the difference becomes excessively large, it is treated as a *time jump*. Time jumps can occur, for example, when the wall clock is adjusted by NTP after being out of sync for an extended period. In such cases, the timer resynchronizes with the wall clock and triggers an immediate tick. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
For now, we include one fixture that we will use to mock the `datetime` class imported in the wall clock timer definition, so we can control the current time in the tests (and set the default to the UNIX epoch to make it easier to read times in the tests). We don't use time-machine for 2 reasons: 1. We will also need to use async-solipsism and time-machine also mocks the monotonic timer, sometimes conflicting with async-solipsism. 2. We want to tell only when sleep was called within the wall clock timer module, not in the tests for example. Signed-off-by: Leandro Lucarella <[email protected]>
llucax
requested review from
florian-wagner-frequenz
and removed request for
a team
github-actions
bot
added
part:tests
There was a problem hiding this comment.
Pull Request Overview
This PR adds a wall clock-attached timer (WallClockTimer) for the timeseries resampler that handles discrepancies between wall clock and monotonic time. The timer can detect wall clock drift and time jumps, emitting warnings and resyncing when necessary to maintain accurate timing.
Key changes:
- Implements a configurable wall clock timer with drift tolerance and jump threshold settings
- Provides detailed clock information through
ClocksInfoandTickInfodataclasses - Includes comprehensive test coverage for timer functionality, configuration, and clock behavior
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/frequenz/sdk/timeseries/_resampling/_wall_clock_timer.py |
Core implementation of the wall clock timer, configuration, and clock info classes |
src/frequenz/sdk/timeseries/__init__.py |
Exports the new timer-related classes and existing resampling exceptions |
tests/timeseries/_resampling/wall_clock_timer/test_timer_basic.py |
Basic functionality tests for timer initialization, start/stop, and string representations |
tests/timeseries/_resampling/wall_clock_timer/test_config.py |
Configuration validation tests for timer settings and parameter validation |
tests/timeseries/_resampling/wall_clock_timer/test_clocksinfo.py |
Tests for clock information calculations and drift detection |
tests/timeseries/_resampling/wall_clock_timer/util.py |
Test utilities for datetime comparisons and clock information mocking |
tests/timeseries/_resampling/wall_clock_timer/conftest.py |
Test fixtures for datetime and asyncio sleep mocking |
tests/timeseries/_resampling/wall_clock_timer/__init__.py |
Empty test package initialization file |
tests/timeseries/_resampling/__init__.py |
Empty test package initialization file |
Comments suppressed due to low confidence (2)
tests/timeseries/_resampling/wall_clock_timer/util.py:56
- The class name
approx_timedoesn't follow Python naming conventions. It should beApproxTimeto match PascalCase for class names.
class approx_time(ApproxBase): # pylint: disable=invalid-name, abstract-method
tests/timeseries/_resampling/wall_clock_timer/util.py:183
- The class name
matches_redoesn't follow Python naming conventions. It should beMatchesReto match PascalCase for class names.
class matches_re: # pylint: disable=invalid-name
This commit adds a utility module for testing the wall clock timer. The utility functions include a method to convert various time-related types (datetime, timedelta, float) to seconds, and a function to get the current wall clock time from the mocked datetime (when the `datetime_mock` fixture is used). Signed-off-by: Leandro Lucarella <[email protected]>
We need this to be able to import then new `util.py` module in the tests. Signed-off-by: Leandro Lucarella <[email protected]>
These tests only test the basic functionality, like construction, string representation, etc. The ticking behaviour test will be done in a sepate file in a follow-up commit. Signed-off-by: Leandro Lucarella <[email protected]>
This tool can compare datetime or timedelta objects approximately, like pytest.approx(). It uses an absolute 1ms tolerance by default. Signed-off-by: Leandro Lucarella <[email protected]>
Like pytest.approx(), but compares `TickInfo` objects. It uses an absolute 1ms tolerance by default. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
We monitor only the calls to sleep inside the wall clock time module, so we can assert those calls independently from what happens in the tests themselves, for example. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
The `TimeDriver` class encapsulates the necessary mocks for `datetime.datetime` and provides methods to manipulate wall clock time during tests. This is particularly useful for testing components that rely on both wall clock time (which can be adjusted by the system) and monotonic time (which should always move forward). We also register the util module so pytest rewrite asserts, as it uses asserts to verify some stuff behaves as expected, and it is nicer to get those asserts as test failures instead of programming errors. Signed-off-by: Leandro Lucarella <[email protected]>
This check is also performed by `flake8`. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
These tests cover the behavior of the WallClockTimer when it is ticking, including how it handles wall clock and monotonic clock synchronization, drift, and jumps. The tests include scenarios where the clocks are in sync, where there is a constant drift (both forward and backward), and where there are jumps in the wall clock time. The tests also verify that the timer adjusts its sleep time accordingly and that it logs appropriate warnings when the wall clock time drifts too much from the monotonic time. These tests cover for the most common cases, but are not comprehensive. With the current test infrastructure, it should be possible to add tests for more scenarios fairly easily in the future as needed. Signed-off-by: Leandro Lucarella <[email protected]>
Testing the ticking behavior is very complicated in itself, and when there are failures, it is very difficult to understand what is happening and where the differences between the expected and actual tick times are. To help with this, we add a custom comparison for `TickInfo` objects that provides a detailed report of the differences between the expected and actual tick times, including the sleep information for each sleep. This uses the special `pytest` hook `pytest_assertrepr_compare`: https://docs.pytest.org/en/stable/reference/reference.html#pytest.hookspec.pytest_assertrepr_compare Signed-off-by: Leandro Lucarella <[email protected]>
We expose defaults and the `Sink` and `Source` types because they are exposed to users. Signed-off-by: Leandro Lucarella <[email protected]>
The proper location should be `timeseries` as one might need to use this class not only when using the actor. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
This configuration object is exactly the same as `ResamplerConfig` but replaces the `align_to` field with a new `timer_config` field that holds a `WallClockTimerConfig` instance (or `None` to use the default). It still inherits from `ResamplerConfig` to make the migration easier. Because the `ResamplingFunction` also takes the config, a `ResamplingFunction2` is also added to keep backwards compatibility with any custom resampling function users might be using. Signed-off-by: Leandro Lucarella <[email protected]>
When passing a `ResamplerConfig2` to the resampler, it will use a `WallClockTimer` instead of a monotonic `Timer` to trigger the resampling. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
When using `# pylint: disable=...` in its own line, it is disabled for the rest of the scope, not only for the next line. Signed-off-by: Leandro Lucarella <[email protected]>
We just parametrize the existing tests to also test the resampler when using the `ResamplerConfig2` class (i.e. the wall clock timer). Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
We want to encourage new users to use the new wall clock timer, so it better to use the new config object in examples. Signed-off-by: Leandro Lucarella <[email protected]>
In tests where there are no behavior differences between using the old monotonic timer and the new wall clock timer, we just use the wall clock timer, as we plan to deprecate the other timer soon. Signed-off-by: Leandro Lucarella <[email protected]>
These tests are very tricky and there seems to be some inconsistency between the monotonic and wall clock, the wall clock timer have less samples in the buffer at the end. This is probably some border condition because of the way we fake time. We also had to tune how to do wall clock time shifting to make sure the shift occurs before the timer wakes up. Finally we adjusted sample sending so it always sends an odd number of samples to make sure the resample is triggered at the points we expect it, otherwise again we could end up in border condition that were not the same for both timers. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
github-actions
bot
added
part:docs
shsms
reviewed
Aug 18, 2025
Add comment about using __setattr__ to bypass frozen dataclass restriction.
I just finished going through the comments on your initial review. About tests, there is a bit of black magic to be able to get nice pytest output. Most of it was done by AI, and I tried to make it as clean as possible but also didn't spend a lot of time on cleaning it and making it super nice, as it is just a testing tool. I think I mentioned this somewhere else, but letting you know just in case. |
shsms
reviewed
Aug 26, 2025
|
Will pick up from "Add custom comparison for TickInfo objects for pytest" tomorrow. |
shsms
approved these changes
Aug 27, 2025
Comment on lines
+97
to
+103
| # We are dealing these fields dynamically, so we make a sanity check here to make sure | ||
| # if something changes, we can catch it early instead of getting some cryptic errors | ||
| # later, deep in the code. | ||
| assert set(TickInfo.__dataclass_fields__.keys()) == { | ||
| "expected_tick_time", | ||
| "sleep_infos", | ||
| } |
There was a problem hiding this comment.
What does this mean: "we are dealing these fields ..."?
github-project-automation
bot
moved this from To do
to Review approved
in Python SDK Roadmap
|
I'll approve again after you squash |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This timer uses the wall clock to trigger ticks and handles discrepancies between the wall clock and monotonic time. Since sleeping is performed using monotonic time, differences between the two clocks can occur.
When the wall clock progresses slower than monotonic time, it is referred to as compression (wall clock time appears in the past relative to monotonic time). Conversely, when the wall clock progresses faster, it is called expansion (wall clock time appears in the future relative to monotonic time). If these differences exceed a configured threshold, a warning is emitted.
If the difference becomes excessively large, it is treated as a time jump. Time jumps can occur, for example, when the wall clock is adjusted by NTP after being out of sync for an extended period. In such cases, the timer resynchronizes with the wall clock and triggers an immediate tick.