score/mw/diag: added C++ API for diag-lib by MonikaKumari26 · Pull Request #4 · eclipse-score/inc_diagnostics

MonikaKumari26 · 2026-06-09T13:05:21Z

Description

This PR introduces a C++ implementation of the diagnostics API, mirroring the design and implementation provided in the Rust version.

References

Rust API Design: Initial Rust API design for diag-lib #1
Rust Implementation: Implementation of initial Rust API Design for diag-lib #2

github-actions · 2026-06-09T13:09:08Z

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: 8ef0faff-1eae-426a-a01e-aa5edc139239
Computing main repo mapping: 
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: 
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 (29 packages loaded, 9 targets configured)

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

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

Analyzing: target //:license-check (122 packages loaded, 809 targets configured)

Analyzing: target //:license-check (137 packages loaded, 1824 targets configured)

Analyzing: target //:license-check (143 packages loaded, 2472 targets configured)

Analyzing: target //:license-check (143 packages loaded, 2472 targets configured)

Analyzing: target //:license-check (143 packages loaded, 2472 targets configured)

Analyzing: target //:license-check (146 packages loaded, 4479 targets configured)

Analyzing: target //:license-check (146 packages loaded, 4479 targets configured)

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

Analyzing: target //:license-check (148 packages loaded, 4723 targets configured)

Analyzing: target //:license-check (148 packages loaded, 4723 targets configured)

Analyzing: target //:license-check (148 packages loaded, 4723 targets configured)

Analyzing: target //:license-check (148 packages loaded, 4723 targets configured)

INFO: Analyzed target //:license-check (149 packages loaded, 6658 targets configured).
[9 / 13] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 0s disk-cache
INFO: From Generating Dash formatted dependency file ...:
INFO: Successfully converted 2 packages from Cargo.lock to bazel-out/k8-fastbuild/bin/formatted.txt
[10 / 13] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 1s disk-cache, processwrapper-sandbox
[12 / 13] 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: 204.101s, Critical Path: 2.74s
INFO: 13 processes: 9 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 13 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-09T13:10:44Z

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

rpreddyhv

initial review findings

rpreddyhv · 2026-06-23T10:57:11Z

+
+_VISIBILITY = [
+    "//score:__subpackages__",
+    "//platform:__subpackages__",


question: What is the platform here?

rpreddyhv · 2026-06-23T10:59:46Z

+cc_library(
+    name = "simple_operation",
+    hdrs = ["simple_operation.h"],
+    include_prefix = "score/mw/diag",
+    visibility = _VISIBILITY,
+    deps = [
+        ":operation",
+    ],
+)


nitpick: This should be moved up into libraries section.

rpreddyhv · 2026-06-23T11:00:10Z

+cc_library(
+    name = "uds_adapters",
+    hdrs = ["uds_adapters.h"],
+    include_prefix = "score/mw/diag",
+    visibility = _VISIBILITY,
+    deps = [
+        ":data_resource",
+        ":simple_operation",
+        ":uds",
+    ],
+)


nitpick: This should be moved up into the libraries section.

rpreddyhv · 2026-06-23T11:18:00Z

question: How is filename result.h relevant here?
suggestion: This file has multiple types, consider splitting it into multiple small files with relevant types.

will split this into focused sub-headers for byte_types, attributes, nrc, payloads..

rpreddyhv · 2026-06-23T11:19:46Z

+  public:
+    using value_type     = std::pair<Key, Value>;
+    using iterator       = typename std::vector<value_type>::iterator;
+    using const_iterator = typename std::vector<value_type>::const_iterator;


issue: naming style is not consistent with rest of the file:

using ByteVector = std::vector<std::byte>; using ByteView = score::cpp::span<const std::byte>;

It is intentional as like other C++ container

rpreddyhv · 2026-06-23T12:25:28Z

+/* DataCategory                     */
+/************************************/
+
+/// cf. ISO 17978-3:2025 Section 7.9.1, Table 70


question: what is cf here?

cf. is a Latin abbreviation for "see"

rpreddyhv · 2026-06-23T12:30:59Z

issue: This file requires refactoring to converting DateCategory into a generic type and Specialise it for different Kinds.

Keeping this PR focused; will address in a dedicated PR..

rpreddyhv · 2026-06-23T12:32:06Z

+    DataCategory               item;
+    std::optional<std::string> category_translation_id;


Suggested change

DataCategory item;

std::optional<std::string> category_translation_id;

DataCategory category;

std::optional<TranslationId> translation_id;

rpreddyhv · 2026-06-23T12:41:41Z

issue: In general, use more DSL for all types like std::string. For Example:

std::string id; std::string name; std::optional<std::string> translation_id; std::optional<std::vector<std::string>> groups;

can be written as

struct StringLike { constexpr static std::string value }; using Id = StringLike; using Name = StringLike; using Group = StringLike; using TransactionId = StringLike; struct OptionalTransactionId { constexpr static std::optional<TransactionId> value }; struct OptionalGroups { constexpr static std::optional<std::vector<Group>> value }; Id id; Name name; OptionalTransactionId transaction_id; OptionalGroups groups

Aliases added... however the proposed struct StringLike { constexpr static std::string value } pattern seems not viable here, requires C++20

rpreddyhv · 2026-06-23T12:45:40Z

+
+/// Result type for DataResource::write() — success (monostate) or a DataError.
+/// Corresponds to Rust's WriteValueHandle wrapping Result<(), DataError>.
+using WriteValueResult = std::variant<std::monostate, sovd::DataError>;


Suggested change

using WriteValueResult = std::variant<std::monostate, sovd::DataError>;

using WriteValueReply = std::variant<std::monostate, sovd::DataError>;

to be consistent with ReadValueReply.

FScholPer · 2026-06-24T12:02:20Z

Toolchain setup required to compile this PR

The C++ API introduced in this PR depends on @score_baselibs types (score/language/futurecpp, score/result) which require the S-CORE certified toolchains to build correctly.

I've opened #5 (build: adopt baselibs-style toolchain settings) which aligns .bazelrc and MODULE.bazel with the configuration used in eclipse-score/baselibs.

What was blocking the build

bazel build //... failed with: [email protected] depends on [email protected] with compatibility level 2, but <root> depends on [email protected] with compatibility level 1
No GCC 12.2.0 or Ferrocene toolchains were registered, so there was no platform that could satisfy the score_baselibs constraint requirements

After applying #5

bazel build --config=bl-x86_64-linux //score/...
# INFO: Found 19 targets...
# INFO: Build completed successfully, 77 total actions

Also noted: two test files without BUILD entries

simple_operation_test.cpp and uds_adapters_test.cpp exist in score/mw/diag/api_cpp/ but have no corresponding cc_test targets in the BUILD file — those tests won't run. Please add the missing targets to the BUILD file in this PR.

stmuench · 2026-06-24T12:26:36Z

that would not be correct. instead, result type from score-baselibs repo must be used instead

Actually our Error type is score::mw::diag::sovd::Error (with NRC + SOVD fields), while score::Result carries score::result::Error. These are different error types. So is that OK if Error should be replaced with or unified with score::result::Error.

Clarification needed: does this mean our sovd::Error type should be expressed as score::result::Error (wrapping the error code via MakeError/error domain), so that Result becomes score::Result directly? Or should score::Result be aliased with our error type(score::mw::diag::sovd::Error) via the details::expected constructor?

Please explain the intended integration pattern so I can align sovd_error.h accordingly.

FScholPer · 2026-06-24T12:55:54Z

High-priority validation update after reproducing locally:

Build blocker (resolved by toolchain setup)

Reproduced on PR head: bazel build //... failed because @score_baselibs was not resolvable from MODULE/.bazelrc settings.
After applying the same toolchain/module setup used in PR build: adopt baselibs-style toolchain settings #5 (bl-x86_64-linux + score_baselibs deps), Bazel analysis/build progressed correctly.

Test coverage gap in BUILD (new tests not wired)

New files exist:
- score/mw/diag/api_cpp/simple_operation_test.cpp
- score/mw/diag/api_cpp/uds_adapters_test.cpp
Added missing cc_test targets locally to execute them.

Next true code-level blocker after wiring tests

//score/mw/diag/api_cpp:uds_adapters_test fails at link time with undefined references to:
- DataResourceAdapter::{with_rdbi,with_wdbi,read,write}
- RoutineControlAdapter::{RoutineControlAdapter,start,stop,completion_percentage}
Root cause: score/mw/diag/api_cpp/uds_adapters.h declares non-inline methods, but no corresponding implementation translation unit is present (uds_adapters.cpp missing).

Recommended unblock order:

Land/apply toolchain + MODULE/.bazelrc setup from PR build: adopt baselibs-style toolchain settings #5.
Add missing cc_test targets for simple_operation_test and uds_adapters_test in score/mw/diag/api_cpp/BUILD.
Add implementations for adapter methods (either inline in header or in a new uds_adapters.cpp and add srcs to the uds_adapters library target).
Re-run:
- bazel build --config=bl-x86_64-linux //score/mw/diag/api_cpp:all
- bazel test --config=bl-x86_64-linux //score/mw/diag/api_cpp:all

score/mw/diag: added C++ API for diag-lib

648a9bd

score/mw/diag: added C++ API for diag-lib

9d3c635

MonikaKumari26 marked this pull request as ready for review June 10, 2026 08:13

rama492 mentioned this pull request Jun 12, 2026

created initial CPP API draft for user applications eclipse-opensovd/opensovd#75

Closed

4 tasks

MonikaKumari26 added 3 commits June 16, 2026 19:46

score/mw/diag: added score baselib dependency for result and ByteView

c725271

score/mw/diag: BUILD file lint fix

fdc5671

extend diagnostic API with DTC, UDS service and type refinements

d373f66

rpreddyhv suggested changes Jun 23, 2026

View reviewed changes

MonikaKumari26 mentioned this pull request Jun 23, 2026

score/mw/diag: C++ Implementation for diag-lib MonikaKumari26/inc_diagnostics#1

Open

Merge branch 'main' into cpp_api_implementation_for_diag_lib

513e113

FScholPer mentioned this pull request Jun 24, 2026

build: adopt baselibs-style toolchain settings #5

Open

stmuench reviewed Jun 24, 2026

View reviewed changes

		DataCategory item;
		std::optional<std::string> category_translation_id;

	using WriteValueResult = std::variant<std::monostate, sovd::DataError>;
	using WriteValueReply = std::variant<std::monostate, sovd::DataError>;

Uh oh!

Conversation

MonikaKumari26 commented Jun 9, 2026

Description

References

Uh oh!

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

Uh oh!

rpreddyhv left a comment

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!

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!

FScholPer commented Jun 24, 2026

Toolchain setup required to compile this PR

What was blocking the build

After applying #5

Also noted: two test files without BUILD entries

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

FScholPer commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions Bot commented Jun 9, 2026 •

edited

Loading