vehicle_time: Deliver VehicleClock callbacks from a worker thread by florianfueller · Pull Request #215 · eclipse-score/time

florianfueller · 2026-09-08T18:24:24Z

The Set/Unset callback methods of VehicleClockBackendImpl were no-ops.
Callbacks are now delivered by a worker thread that polls the
TimeDaemon shared memory every 50 ms:

The thread starts with the first registered callback after Init()
and runs until the backend is destroyed. While no callback is
registered it sleeps.
A callback receives the first snapshot polled after its registration
and afterwards only changes of its part of the snapshot:
TimeSlaveSyncData and PDelayMeasurementData on any change,
VehicleTimeStatus only when the flags change (rate deviation
excluded).
Set/Unset are safe against in-flight invocations, also from inside a
callback; Unset returns only after a running invocation finished.

Add unit tests, update the docs and remove the "not yet delivered"
warnings.

closes #59 (improvement ticket)

github-actions · 2026-09-08T18:25:19Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.6.0) and connecting to it...
INFO: Invocation ID: 1ad62f5b-ca6f-4164-b6bd-dda3ec582210
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'platforms', the root module requires module version platforms@1.0.0, but got platforms@1.1.0 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'score_tooling', the root module requires module version score_tooling@2.2.1, but got score_tooling@2.2.2 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'rules_oci', the root module requires module version rules_oci@2.2.7, but got rules_oci@2.3.0 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
WARNING: For repository 'score_devcontainer', the root module requires module version score_devcontainer@1.8.0, but got score_devcontainer@1.11.1 in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 3 packages loaded
WARNING: Target pattern parsing failed.
ERROR: Skipping '//:license-check': no such target '//:license-check': target 'license-check' not declared in package '' defined by /home/runner/work/time/time/BUILD
ERROR: no such target '//:license-check': target 'license-check' not declared in package '' defined by /home/runner/work/time/time/BUILD
INFO: Elapsed time: 16.820s
INFO: 0 processes.
ERROR: Build did NOT complete successfully
ERROR: Build failed. Not running target

lavrovvalera · 2026-10-07T16:03:27Z

-    const auto status_flags = ConvertPtpStatus(snapshot.value().status);
-    score::cpp::ignore =
-        status_slot_.InvokeIfChanged(status_flags, VehicleTimeStatus{status_flags, snapshot.value().rate_deviation});
+    score::cpp::ignore = sync_data_slot_.TryDeliverChangedData(ConvertSyncData(snapshot.value().sync_fup_data));


why do we need still to ignore the result?
regarding the returning result.
I think, the point, the callback was executed or nto is indeed not interesting.
but if the callback will return some result and we can react on it - that might be interesting.
but do we need this thing?
one of the use case - the callback could return if the registration shall still be hold.
so, if it will return true - continue calling it, false - drop the subscription.

if not, I would propose to drop the return at all.

Return dropped.

lavrovvalera · 2026-10-07T16:03:44Z

+
+void SvtCallbackDispatcher::PollAndDispatch() noexcept
+{
+    const auto snapshot = svt_receiver_->Receive();


then we need to investigate before we will merge it

lavrovvalera · 2026-10-07T16:06:29Z

+template <typename Timebase>
+bool IsSameForDelivery(const TimeSlaveSyncData<Timebase>& first, const TimeSlaveSyncData<Timebase>& second) noexcept
+{
+    const bool same_precise_origin_timestamp = (first.precise_origin_timestamp == second.precise_origin_timestamp);


why the default instantiation doesn't fit here?

Implemented operator==

lavrovvalera · 2026-10-07T16:06:35Z

+bool IsSameForDelivery(const PDelayMeasurementData<Timebase>& first,
+                       const PDelayMeasurementData<Timebase>& second) noexcept
+{
+    const bool same_request_origin_timestamp = (first.request_origin_timestamp == second.request_origin_timestamp);


why the default instantiation doesn't fit here?

Implemented operator==

lavrovvalera · 2026-10-07T16:07:16Z

-/// @brief Thread-safe holder for a single move-only callback that is invoked from a dedicated worker thread
+/// @brief Delivery comparison for types that already provide @c operator==.
+template <typename Value>
+bool IsSameForDelivery(const Value& first, const Value& second) noexcept


Not sure, I can understand the function name

Changed to IsSame

lavrovvalera · 2026-10-07T16:11:23Z

    /// @return @c true if the callback was invoked, @c false if none is installed or @p data is unchanged.
-    template <typename Argument>
-    bool InvokeIfChanged(const Data& data, const Argument& argument) noexcept
+    bool TryDeliverChangedData(const Data& data) noexcept


is it a delivery?
Maybe TryToEnvoke() or somehting?
also the user of this function doesn't know, what the "new" data is.
he jsut has the "data"

Renamed to TryToEnvoke

lavrovvalera · 2026-10-07T16:11:57Z

+    std::recursive_mutex mutex_;
    std::shared_ptr<Callback> callback_{};
    std::optional<Data> last_data_{};
+    // Mirrors callback_ != nullptr. Written only under mutex_, read lock-free by IsSet().


I don't think, we need this comment

github-actions · 2026-10-08T12:12:26Z

Documentation preview for this pull request is available at:
pr-215: https://eclipse-score.github.io/time/pr-215/

lavrovvalera · 2026-10-12T07:09:38Z

-            // No write was in progress when the copy started and none started since
-            if (exit_cnt_before_read == entry_cnt_after_read)
+            // Even: no write in progress
+            if ((seq_before_read & 1U) == 0U)


Create a very small, tiny but self-explaining function.
you will not need the comment then.

lavrovvalera · 2026-10-12T07:36:06Z

            {
-                return read_data;
+                // Copy the payload
+                read_data = shared_memory_data_->data_;


Shall we set something another atomic variable to block the writer for moment here?
or it wil l be anyway compensated by +1 and +2 done by writer?

The counter does the job: An in-progress write makes it odd and a complete write makes it even again. So the reader's final check fails and it retries. A lock would make the writer wait on slow readers.

What still remeins is a data race. The reader and the writer could still access the same memory location at the same time. This is undefined behavior. But we should solve this issue in a dedicated ticket.

lavrovvalera · 2026-10-12T07:47:15Z

+        shared_memory_data_->seq_.store(seq + 1U, std::memory_order_relaxed);

-        // Copy the non-atomic payload
+        // Copy the payload


do we need this comment?

I can remove it

lavrovvalera · 2026-10-12T07:54:49Z

+    wakeup_requested_ = true;
+    StartWorkerIfReadyLocked();
+    // Also delivers the first snapshot promptly to a fresh registration instead of after a full poll interval.
+    worker_wakeup_.notify_all();


if the thread doesn't wait for itright now, the even will be missed

Yes notify_all() is lost when nothing is waiting, but the callback registration already happened in SetCallback and with the next poll the value will be delivered. The notify_all() here is for waking up the thread if it is currently waiting.

The Set/Unset callback methods of VehicleClockBackendImpl were no-ops. Callbacks are now delivered by a worker thread that polls the TimeDaemon shared memory every 50 ms: - The thread starts with the first registered callback after Init() and runs until the backend is destroyed. While no callback is registered it sleeps. - A callback receives the first snapshot polled after its registration and afterwards only changes of its part of the snapshot: TimeSlaveSyncData and PDelayMeasurementData on any change, VehicleTimeStatus only when the flags change (rate deviation excluded). - Set/Unset are safe against in-flight invocations, also from inside a callback; Unset returns only after a running invocation finished. Add unit tests, update the docs and remove the "not yet delivered" warnings. Refs eclipse-score#59 Signed-off-by: Florian Füller <5454068+florianfueller@users.noreply.github.com>

github-project-automation Bot added this to TIM - Time FT Sep 8, 2026

github-project-automation Bot moved this to In Progress in TIM - Time FT Sep 8, 2026

florianfueller had a problem deploying to workflow-approval September 8, 2026 18:24 — with GitHub Actions Error

florianfueller force-pushed the feature/59-vehicle-clock-callback-delivery branch from d4f4e67 to 143d1fb Compare September 9, 2026 07:57

florianfueller had a problem deploying to workflow-approval September 9, 2026 07:58 — with GitHub Actions Error

florianfueller force-pushed the feature/59-vehicle-clock-callback-delivery branch from 143d1fb to d447876 Compare September 9, 2026 11:18

florianfueller had a problem deploying to workflow-approval September 9, 2026 11:18 — with GitHub Actions Error

florianfueller force-pushed the feature/59-vehicle-clock-callback-delivery branch from d447876 to a816147 Compare September 9, 2026 12:37

florianfueller had a problem deploying to workflow-approval September 9, 2026 12:38 — with GitHub Actions Error

florianfueller marked this pull request as ready for review September 9, 2026 12:38

florianfueller requested a review from lavrovvalera September 9, 2026 12:39

florianfueller had a problem deploying to workflow-approval September 9, 2026 18:58 — with GitHub Actions Error

florianfueller had a problem deploying to workflow-approval September 10, 2026 13:09 — with GitHub Actions Error

florianfueller force-pushed the feature/59-vehicle-clock-callback-delivery branch from c811e27 to 041c9b8 Compare September 23, 2026 06:56

florianfueller had a problem deploying to workflow-approval September 23, 2026 06:56 — with GitHub Actions Error

florianfueller force-pushed the feature/59-vehicle-clock-callback-delivery branch from 041c9b8 to 4448f49 Compare September 23, 2026 07:42

florianfueller had a problem deploying to workflow-approval September 23, 2026 07:42 — with GitHub Actions Error

florianfueller force-pushed the feature/59-vehicle-clock-callback-delivery branch from 4448f49 to 756eaad Compare September 23, 2026 08:06

florianfueller had a problem deploying to workflow-approval September 23, 2026 08:06 — with GitHub Actions Error

florianfueller force-pushed the feature/59-vehicle-clock-callback-delivery branch from 756eaad to 28b62ff Compare September 23, 2026 08:32

florianfueller had a problem deploying to workflow-approval October 7, 2026 09:33 — with GitHub Actions Error

florianfueller had a problem deploying to workflow-approval October 7, 2026 10:24 — with GitHub Actions Error

florianfueller had a problem deploying to workflow-approval October 7, 2026 10:30 — with GitHub Actions Error

florianfueller had a problem deploying to workflow-approval October 7, 2026 10:33 — with GitHub Actions Error

florianfueller had a problem deploying to workflow-approval October 7, 2026 10:39 — with GitHub Actions Error

florianfueller had a problem deploying to workflow-approval October 7, 2026 11:26 — with GitHub Actions Error

florianfueller had a problem deploying to workflow-approval October 7, 2026 11:41 — with GitHub Actions Error

florianfueller had a problem deploying to workflow-approval October 7, 2026 11:44 — with GitHub Actions Error

florianfueller had a problem deploying to workflow-approval October 7, 2026 11:54 — with GitHub Actions Error

florianfueller force-pushed the feature/59-vehicle-clock-callback-delivery branch from e5731ec to 150c89a Compare October 7, 2026 13:03

florianfueller had a problem deploying to workflow-approval October 7, 2026 13:04 — with GitHub Actions Error

lavrovvalera reviewed Oct 7, 2026

View reviewed changes

florianfueller had a problem deploying to workflow-approval October 7, 2026 19:46 — with GitHub Actions Error

florianfueller force-pushed the feature/59-vehicle-clock-callback-delivery branch from 22c6f71 to 0eee42c Compare October 8, 2026 11:37

florianfueller had a problem deploying to workflow-approval October 8, 2026 11:37 — with GitHub Actions Error

lavrovvalera reviewed Oct 12, 2026

View reviewed changes

lavrovvalera approved these changes Oct 13, 2026

View reviewed changes

umaucher approved these changes Oct 13, 2026

View reviewed changes

Conversation

florianfueller commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Oct 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

florianfueller Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch was successfully deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

florianfueller commented Sep 8, 2026 •

edited

Loading

github-actions Bot commented Sep 8, 2026 •

edited

Loading

florianfueller Oct 12, 2026 •

edited

Loading