Replace mmap-based IPC with score_baselibs SharedMemoryFactory in ts_client by gordon9901 · Pull Request #54 · eclipse-score/inc_time

gordon9901 · 2026-06-12T10:10:41Z

Summary

Replaces the hand-rolled POSIX shm_open/mmap implementation in ts_client
with score::memory::shared::SharedMemoryFactory from score_baselibs, and
wires up a new SvtHandlerShm timebase handler in TimeDaemon.

ts_client (`score/ts_client/`)

GptpIpcPublisher / GptpIpcReceiver: remove direct shm_open, mmap,
ftruncate, close calls; use SharedMemoryFactory::Create /
SharedMemoryFactory::Open instead. Memory region is now constructed via
ISharedMemoryResource::construct<GptpIpcRegion>().
SharedMemoryFactory::Remove + RemoveStaleArtefacts replace the raw
shm_unlink in publisher init.
Header guards updated to drop the redundant _SRC_ segment.

TimeDaemon (`score/time_daemon/src/application/svt/`)

New SvtHandlerShm (factory_shm.h/.cpp, svt_handler_shm.h/.cpp):
a TimebaseHandler implementation that reads live gPTP data from shared
memory via GPTPShmMachine and drives the same verification + IPC-publish
pipeline as the existing SvtHandler.

fixes (#41)

github-actions · 2026-06-12T10:11:33Z

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.3.0) and connecting to it...
INFO: Invocation ID: 75bec5d3-5f08-4ec3-8e86-12b20d611d73
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: 
Computing main repo mapping: 
Computing main repo mapping: 
WARNING: For repository 'aspect_rules_lint', the root module requires module version [email protected], but got [email protected] in the resolved dependency graph. Please update the version in your MODULE.bazel or set --check_direct_dependencies=off
Computing main repo mapping: 
Loading: 
Loading: 2 packages loaded
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Loading: 2 packages loaded
    currently loading: 
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)
Analyzing: target //:license-check (3 packages loaded, 0 targets configured)

Analyzing: target //:license-check (91 packages loaded, 9 targets configured)

Analyzing: target //:license-check (157 packages loaded, 5785 targets configured)

Analyzing: target //:license-check (169 packages loaded, 8049 targets configured)

Analyzing: target //:license-check (169 packages loaded, 8049 targets configured)

Analyzing: target //:license-check (172 packages loaded, 9937 targets configured)

Analyzing: target //:license-check (173 packages loaded, 10058 targets configured)

Analyzing: target //:license-check (173 packages loaded, 10058 targets configured)

INFO: Analyzed target //:license-check (174 packages loaded, 10184 targets configured).
[10 / 16] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 0s disk-cache, processwrapper-sandbox ... (2 actions, 1 running)
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 2 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
[15 / 16] [Prepa] Building license.check.license_check.jar ()
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 33.774s, Critical Path: 1.87s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

github-actions · 2026-06-12T10:15:24Z

The created documentation from the pull request is available at: docu-html

BjoernAtBosch · 2026-06-15T13:32:40Z

 ********************************************************************************/
-#ifndef SCORE_TS_CLIENT_SRC_GPTP_IPC_H
-#define SCORE_TS_CLIENT_SRC_GPTP_IPC_H
+#ifndef SCORE_TS_CLIENT_GPTP_IPC_H


Here and everywhere: Why doing this include guard renaming?

The SRC segment encodes the physical directory path (ts_client/src/), which is an internal layout detail. Include guards should reflect the logical module name, not the filesystem structure — hence SCORE_TS_CLIENT_* rather than SCORE_TS_CLIENT_SRC_*. All 6 headers under ts_client/src/ were updated consistently.

Yes, I also see this. But I'm struggeling with the point, that the "physical" structure is also visible in the includes, e.g.

#include "score/ts_client/src/gptp_ipc_channel.h"

which then should also be

#include "score/ts_client/gptp_ipc_channel.h"

instead. But that's how it is in Bazel.

If we change it here, we would be inconsistent in the overall module and should change it everywhere. (Maybe not now)

Maybe there is some S-CORE convention. Lets check ...

Checked the lifecycle repo — it uses strip_include_prefix + include_prefix in BUILD to remap physical paths to logical ones, so guards and include paths stay consistent. Should we do the same here (strip src/ in the BUILD and update all include sites), or keep it as a follow-up?

BjoernAtBosch · 2026-06-15T13:34:17Z

 * @brief IPC data snapshot written by TimeSlave and read by TimeDaemon.
 *
- * This type is internal to ts_client/src and intentionally decoupled from
+ * This type is internal to libTSClient and intentionally decoupled from


Suggested change

* This type is internal to libTSClient and intentionally decoupled from

* This type is internal to ts_client and intentionally decoupled from

Good catch. This was an oversight — the comment should say ts_client, not libTSClient. Fixed.

BjoernAtBosch · 2026-06-15T13:36:18Z

 {

-/// Default POSIX shared memory name for the gPTP IPC channel.
 constexpr char kGptpIpcName[] = "/gptp_ptp_info";


Suggested change

constexpr char kGptpIpcName[] = "/gptp_ptp_info";

/// Default shared memory name for the gPTP IPC channel.

constexpr char kGptpIpcName[] = "/gptp_ptp_info";

Agreed. The comment was removed because "POSIX" is no longer accurate after switching to SharedMemoryFactory. Restored as /// Default shared memory name for the gPTP IPC channel.

BjoernAtBosch · 2026-06-15T13:38:42Z


-    /// Open and map the shared memory segment (read-only).
-    /// @return true on success.
    bool Init(const std::string& ipc_name = kGptpIpcName);


Suggested change

bool Init(const std::string& ipc_name = kGptpIpcName);

bool Open(const std::string& ipc_name = kGptpIpcName);

Agreed. Open/Close is more semantically precise and forms a natural pair. Renamed in both GptpIpcPublisher and GptpIpcReceiver, including all call sites and test cases.

BjoernAtBosch · 2026-06-15T13:39:42Z


-    /// Create and map the shared memory segment.
-    /// @return true on success.
    bool Init(const std::string& ipc_name = kGptpIpcName);


Suggested change

bool Init(const std::string& ipc_name = kGptpIpcName);

bool Open(const std::string& ipc_name = kGptpIpcName);

Agreed. Open/Close is more semantically precise and forms a natural pair. Renamed in both GptpIpcPublisher and GptpIpcReceiver, including all call sites and test cases.

BjoernAtBosch · 2026-06-15T13:40:01Z

    void Publish(const score::ts::GptpIpcData& data);
-
-    /// Unmap and unlink the shared memory segment.
    void Destroy();


Suggested change

void Destroy();

void Close();

Agreed. Open/Close is more semantically precise and forms a natural pair. Renamed in both GptpIpcPublisher and GptpIpcReceiver, including all call sites and test cases.

This reverts commit 8c6827d.

github-project-automation Bot added this to TIM - Time FT Jun 12, 2026

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

gordon9901 requested a deployment to workflow-approval June 12, 2026 10:10 — with GitHub Actions Waiting

Replace mmap-based IPC with SharedMemoryFactory in ts_client

7c692e8

gordon9901 force-pushed the ecarx_pr2_shm_impl branch from aad9e21 to 7c692e8 Compare June 12, 2026 10:17

gordon9901 requested a deployment to workflow-approval June 12, 2026 10:17 — with GitHub Actions Waiting

BjoernAtBosch suggested changes Jun 15, 2026

View reviewed changes

BlueXMarv3l requested a review from BjoernAtBosch June 17, 2026 08:49

rename Init/Destroy, drop svt_handler_shm

e14098e

gordon9901 requested a deployment to workflow-approval June 17, 2026 09:56 — with GitHub Actions Waiting

This was referenced Jun 22, 2026

ts_client: Create SW component requirements and establish traceability #75

Open

score/ts_client: Create detailed design (comp_dd_sta) #89

Open

Prepare the roadmap for v0.8-v1.0 #100

Open

Revert include guards to match physical include path

8c6827d

gordon9901 requested a deployment to workflow-approval June 24, 2026 05:53 — with GitHub Actions Waiting

Revert "Revert include guards to match physical include path"

ddfbd8b

This reverts commit 8c6827d.

gordon9901 requested a deployment to workflow-approval June 24, 2026 07:36 — with GitHub Actions Waiting

	* This type is internal to libTSClient and intentionally decoupled from
	* This type is internal to ts_client and intentionally decoupled from

	constexpr char kGptpIpcName[] = "/gptp_ptp_info";
	/// Default shared memory name for the gPTP IPC channel.
	constexpr char kGptpIpcName[] = "/gptp_ptp_info";

	bool Init(const std::string& ipc_name = kGptpIpcName);
	bool Open(const std::string& ipc_name = kGptpIpcName);

Uh oh!

Conversation

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

Uh oh!

Summary

ts_client (score/ts_client/)

TimeDaemon (score/time_daemon/src/application/svt/)

Uh oh!

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

Uh oh!

License Check Results

Uh oh!

github-actions Bot commented Jun 12, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BjoernAtBosch Jun 17, 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gordon9901 commented Jun 12, 2026 •

edited

Loading

ts_client (`score/ts_client/`)

TimeDaemon (`score/time_daemon/src/application/svt/`)

github-actions Bot commented Jun 12, 2026 •

edited

Loading

BjoernAtBosch Jun 17, 2026 •

edited

Loading