Refactor Monitor API to public Alive API by NicolasFussberger · Pull Request #229 · eclipse-score/lifecycle

NicolasFussberger · 2026-06-05T14:30:00Z

Refactors the Launch Manager "Monitor" interface to the Alive API.

Example usage:

#include "score/mw/lifecycle/alive.h"

score::mw::lifecycle::Alive alive("instance-name");

while(!shutdownRequested) {
  // Do stuff 
  // ...

  alive.ReportAlive();
}

let alive = alive_rs::Alive::new("my instance").expect("Failed to create alive instance");
alive.report_alive();

Public bazel targets:
//score/launch_manager:alive_cc
//score/launch_manager:alive_rust

Changes:

Rename Monitor class to Alive
Remove the possibility to report different checkpoint ids from the public API
Since this is a public launch manager API, it is moved to new top-level score/launch_manager/alive folder

Open Questions:

Feature Architecture defines method "report_health_status" which seems odd if we are only reporting an alive notification. What was the intention of this interface definition?

Closes: #149

github-actions · 2026-06-05T14:31:03Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.4.2) and connecting to it...
INFO: Invocation ID: 08354aaf-5637-4ef8-9892-17eca1e21929
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (31 packages loaded, 10 targets configured)

Analyzing: target //:license-check (86 packages loaded, 10 targets configured)

Analyzing: target //:license-check (138 packages loaded, 2720 targets configured)

Analyzing: target //:license-check (147 packages loaded, 5559 targets configured)

Analyzing: target //:license-check (158 packages loaded, 8130 targets configured)

Analyzing: target //:license-check (158 packages loaded, 8130 targets configured)

Analyzing: target //:license-check (159 packages loaded, 8254 targets configured)

INFO: Analyzed target //:license-check (163 packages loaded, 10268 targets configured).
[10 / 16] [Prepa] Expanding template external/score_tooling+/dash/tool/formatters/dash_format_converter [for tool]
[14 / 16] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar; 0s disk-cache, processwrapper-sandbox
[15 / 16] Building license.check.license_check.jar (); 0s disk-cache, multiplex-worker
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: 19.870s, Critical Path: 2.44s
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-05T14:35:03Z

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

MaciejKaszynski · 2026-06-08T09:37:02Z

 score_testing_macros.workspace = true
 containers.workspace = true
-monitor_rs = { workspace = true, optional = true }
+alive_rs = { workspace = true, optional = true }


is optional correct here?

I suppose that is optional because there is also a stub implementation for the interface within health_monitor.
But the cargo build is not working for me locally, neither before nor with these changes.

Is the cargo build still in use @pawelrutkaq ?

pawelrutkaq · 2026-06-09T06:18:31Z

+
+private:
+    /// @brief Unique pointer to implementation class of Alive
+    std::unique_ptr<AliveImpl> aliveImplPtr;


this pimpl Alive (AliveImpl), does not sound as any benefit really here.

The benefit of the pimpl is that we can keep the public API minimal.
If I were to remove the AliveImpl class everything that is included there would need to become now part of the public API and those classes are meant to stay internal so that we can freely change them later.

not really in my opinion - public is what is public that is. rest can be private/protected and you change it. pimpl here is just code duplication.

I think there are two big benefits here for using pimpl pattern.

1. Keeping implementation details private

The AliveImpl header includes the following internal headers:
#include "score/mw/launch_manager/alive_monitor/details/ifappl/DataStructures.hpp"
#include "score/mw/launch_manager/alive_monitor/details/ipc/IpcClient.hpp"
#include "score/mw/launch_manager/alive_monitor/details/logging/PhmLogger.hpp"
which again include other internal headers.

When removing the AliveImpl class, those headers would go to the Alive header and thus be required to be part of the public API (+all their transitive headers).
Those are all implementation details that should stay private, otherwise we risk easily breaking users when we change those classes in the future. Furthermore, users may start relying on these implementation details (e.g. specific shared memory layout as defined in DataStructures.hpp).

2. ABI compatibility

Size and layout of Alive class will stay unchanged. This can become important when delivering score stack not as bazel modules but as a set of precompiled binaries.

While both are clear benefits in my opinion I would say that the first benefit is the major one.
I don't see why we would expose all these implementation details as part of the API if we can avoid it.
I would say the cost of code duplication is rather small here, compared to the benefit.

pawelrutkaq · 2026-06-09T06:20:37Z

    /// @brief Reports an occurrence of a Checkpoint
    /// @param [in] f_checkpointId   Checkpoint identifier.
-    void ReportCheckpoint(Checkpoint f_checkpointId) const noexcept(true);
+    void ReportCheckpoint(std::uint32_t f_checkpointId) const noexcept(true);


Two things: First i dont see any beenfit having Alive and AliveImpl. Seconds is that Impl provides an extended interface (checkpoint id instead just alive notification), why?

Most of the implementation still deals with checkpoints.
I started removing this from the public API as a first step. The rest of the code is yet to be simplified. I did not want to propagate this change here as it would result in massive amount of changes.
But of course you are right, there numbered checkpoint is already obsolete when there is only an alive notification

so shall we link some ticket for cleanu p ?

I created a ticket here #251

ramceb · 2026-06-10T08:35:06Z

FT-Team meeting: 2026-06-10: The report_health_status api shall also provide the possibility to report "not ok" state to shorten FTTI times in some use-cases.

NicolasFussberger · 2026-06-10T13:20:13Z

FT-Team meeting: 2026-06-10: The report_health_status api shall also provide the possibility to report "not ok" state to shorten FTTI times in some use-cases.

Okay understood. Then I guess the C++ API would look like this:

enum class HealthStatus : uint8_t {
  kOk,
  kNotOk
};

class Alive {
  public:
    void ReportHealthStatus(HealthStatus status);
};

Though I am also wondering if having a separate method for the failure reporting would be more explicit.
Something like:

class Alive {
  public:
    void ReportAlive();
    void ReportFailure();
};

What do you think @pawelrutkaq @ramceb ?

pawelrutkaq · 2026-06-15T09:35:43Z

For me,no matter if enum or static names. Just lets dont have two interfaces for the same ;)

pawelrutkaq · 2026-06-16T06:27:59Z

@@ -0,0 +1,89 @@
+/********************************************************************************


file name lower case, other upper case ?

The other public APIs in launch manager are all with lower case.
So now it is consistent at least to the outside.
We'll have to adapt the private code later

pawelrutkaq · 2026-06-16T06:28:45Z

+    void score_lcm_alive_report_failure(void* instance) noexcept;
+#ifdef __cplusplus
+}
+#endif


this does not need to be part of header or ? can be in cpp ?

you are right, I removed it from the header. Seems enough to have this in the cpp file.

pawelrutkaq · 2026-06-16T06:29:16Z

+
+void Alive::ReportFailure() const noexcept
+{
+    // Not implemented


I added an assertion that fails when this method is called.

pawelrutkaq · 2026-06-16T06:29:42Z

+
+void score_lcm_alive_report_failure(void* instance) noexcept {
+    static_cast<score::mw::lifecycle::Alive*>(instance)->ReportFailure();
+}


missing nullptr checks

Not sure what should be the behavior in case nullptr is passed, this would always be a programming error.
Silently ignoring the nullptr, might hide such programming errors.
I introduced now the baselibs assert macros as nullptr check.

* Introduce nullptr asserts * For non-implement method ReportFailure(), use assert rather than deprecation warning. * Remove C functions from header file. Seems enough to have them in the cpp file

…tor-to-alive

MaciejKaszynski · 2026-06-18T06:50:08Z

+{
+}
+
+Alive::Alive(Alive&& se) noexcept :


I think this move constructor and move assignment can just be = default

done in c7019a4

MaciejKaszynski · 2026-06-18T06:59:35Z

Did a grep for Monitor and found the following that might need to be updated:

./score/launch_manager/daemon/src/alive_monitor/details/factory/FlatCfgFactory.cpp:93 - log mentions health monitor

./score/launch_manager/alive/src/details/AliveImpl.h:28 - the brief should be updated

done in 0fe8760

NicolasFussberger requested a deployment to workflow-approval June 5, 2026 14:30 — with GitHub Actions Waiting

NicolasFussberger had a problem deploying to workflow-approval June 8, 2026 05:19 — with GitHub Actions Error

NicolasFussberger had a problem deploying to workflow-approval June 8, 2026 05:19 — with GitHub Actions Failure

NicolasFussberger marked this pull request as ready for review June 8, 2026 05:21

NicolasFussberger temporarily deployed to workflow-approval June 8, 2026 05:29 — with GitHub Actions Inactive

NicolasFussberger requested review from MaciejKaszynski and pawelrutkaq June 8, 2026 10:24

MaciejKaszynski suggested changes Jun 8, 2026

View reviewed changes

NicolasFussberger mentioned this pull request Jun 8, 2026

Create example app for Alive API #234

Open

NicolasFussberger requested a deployment to workflow-approval June 8, 2026 10:44 — with GitHub Actions Waiting

NicolasFussberger requested a deployment to workflow-approval June 8, 2026 11:59 — with GitHub Actions Waiting

NicolasFussberger requested a review from MaciejKaszynski June 8, 2026 14:08

pawelrutkaq requested changes Jun 9, 2026

View reviewed changes

NicolasFussberger requested a review from pawelrutkaq June 10, 2026 04:10

NicolasFussberger temporarily deployed to workflow-approval June 10, 2026 04:11 — with GitHub Actions Inactive

NicolasFussberger added 4 commits June 11, 2026 11:36

Rename Monitor to Alive API

9a8d537

Make MonitorImplWrapper private

19481b5

Remove instanceSpecifier from Alive API

6aa3aa2

Remove obsolete MonitorImplWrapper class

49772b1

NicolasFussberger added 6 commits June 11, 2026 11:36

Fix bazel formatting

3ee753c

Correct cargo files

447d12d

Add doxygen documentation

31db29a

Update copyright header

2278cde

Add ReportFailure method

a338ba9

Correct name in log message

677968a

NicolasFussberger force-pushed the feature/rename-monitor-to-alive branch from 7a78c98 to 677968a Compare June 11, 2026 09:48

NicolasFussberger requested a deployment to workflow-approval June 11, 2026 09:48 — with GitHub Actions Waiting

pawelrutkaq requested changes Jun 16, 2026

View reviewed changes

Fixes for Alive API

3d0954a

* Introduce nullptr asserts * For non-implement method ReportFailure(), use assert rather than deprecation warning. * Remove C functions from header file. Seems enough to have them in the cpp file

NicolasFussberger requested a deployment to workflow-approval June 16, 2026 15:11 — with GitHub Actions Waiting

NicolasFussberger requested a review from pawelrutkaq June 16, 2026 15:13

anmittag added this to LCM - Lifecycle & Health FT Jun 17, 2026

github-project-automation Bot moved this to Backlog in LCM - Lifecycle & Health FT Jun 17, 2026

anmittag assigned NicolasFussberger Jun 17, 2026

Merge remote-tracking branch 'upstream/main' into feature/rename-moni…

e3b62de

…tor-to-alive

NicolasFussberger had a problem deploying to workflow-approval June 18, 2026 06:33 — with GitHub Actions Error

NicolasFussberger had a problem deploying to workflow-approval June 18, 2026 06:33 — with GitHub Actions Failure

NicolasFussberger requested a deployment to workflow-approval June 18, 2026 06:52 — with GitHub Actions Waiting

MaciejKaszynski suggested changes Jun 18, 2026

View reviewed changes

NicolasFussberger added 2 commits June 18, 2026 09:09

Default move ctor and assignment

c7019a4

More renaming Monitor -> Alive

0fe8760

NicolasFussberger requested a deployment to workflow-approval June 18, 2026 07:20 — with GitHub Actions Waiting

NicolasFussberger requested a review from MaciejKaszynski June 18, 2026 07:21

MaciejKaszynski approved these changes Jun 18, 2026

View reviewed changes

		@@ -0,0 +1,89 @@
		/********************************************************************************

Conversation

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

Uh oh!

Uh oh!

github-actions Bot commented Jun 5, 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 5, 2026

Uh oh!

Uh oh!

Uh oh!

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

1. Keeping implementation details private

2. ABI compatibility

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasFussberger Jun 9, 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!

ramceb commented Jun 10, 2026

Uh oh!

NicolasFussberger commented Jun 10, 2026

Uh oh!

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

Uh oh!

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!

5 participants

NicolasFussberger commented Jun 5, 2026 •

edited

Loading

github-actions Bot commented Jun 5, 2026 •

edited

Loading

NicolasFussberger Jun 9, 2026 •

edited

Loading

pawelrutkaq commented Jun 15, 2026 •

edited

Loading