Moved unit tests to CXX files to test C++ bindings implicitly. by nayakned · Pull Request #117 · COVESA/Open1722

nayakned · 2026-02-19T05:41:07Z

This should avoid issues like #115 in the future.

Signed-off-by: Naresh Nayak <Naresh.Nayak@de.bosch.com>

SebastianSchildt · 2026-02-19T08:19:28Z

By just renaming/forcing a CPP compiler, are we sure we will not have the problem the other way around? I.e. C compiler unhappy?

Signed-off-by: Naresh Nayak <Naresh.Nayak@de.bosch.com>

nayakned · 2026-02-19T09:24:22Z

By just renaming/forcing a CPP compiler, are we sure we will not have the problem the other way around? I.e. C compiler unhappy?

True. The idea was that, since the examples were built using C, we could identify errors there. However, the unit tests are now run using both a C and a C++ compiler. Hopefully, this will flag errors in both.

Copilot

Pull request overview

This PR enhances the test infrastructure by making all unit tests compilable as both C and C++ code to catch C++ binding compatibility issues early. This addresses the root cause of issue #115, where an implicit conversion from uint64_t to an enum type was not caught until C++ compilation.

Changes:

Modified all unit test files to support C++ compilation by wrapping cmocka includes in extern "C" blocks and replacing designated initializers with zero-initialization followed by member assignment
Updated CMakeLists.txt to enable C++ (C++17 standard) and create duplicate test executables compiled as C++ (with -cpp suffix) for each existing test
Added SPDX-License-Identifier to test files that were missing it for better license clarity

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
CMakeLists.txt	Enabled C++ language support and set C++17 as the standard
unit/CMakeLists.txt	Added C++ test executables for all unit tests, creating duplicate tests compiled as C++
unit/test-can.c	Added extern "C" wrapper for cmocka include for C++ compatibility
unit/test-aaf.c	Added extern "C" wrapper and SPDX identifier
unit/test-avtp.c	Added extern "C" wrapper and SPDX identifier
unit/test-crf.c	Added extern "C" wrapper and SPDX identifier
unit/test-cvf.c	Added extern "C" wrapper and SPDX identifier
unit/test-rvf.c	Added extern "C" wrapper, SPDX identifier, and explicit casts for alloca calls
unit/test-tscf.c	Added extern "C" wrapper for cmocka include
unit/test-ntscf.c	Added extern "C" wrapper for cmocka include
unit/test-vss.c	Added extern "C" wrapper, replaced designated initializers with zero-init + assignment, added explicit cast for strlen
unit/test-flexray.c	Added extern "C" wrapper for cmocka include
unit/test-lin.c	Added extern "C" wrapper for cmocka include
unit/test-most.c	Added extern "C" wrapper for cmocka include
unit/test-gpc.c	Added extern "C" wrapper for cmocka include
unit/test-sensor.c	Added extern "C" wrapper for cmocka include
unit/test-udp.c	Added extern "C" wrapper for cmocka include
unit/test-commonheader.c	Added extern "C" wrapper for cmocka include

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-19T09:29:20Z

    Avtp_Vss_SetDatatype(vss_pdu, VSS_STRING);
    Avtp_Vss_SetVssData(vss_pdu, &data);
-    uint8_t len_in_mem[] = {0, strlen(str_value)};
+    uint8_t len_in_mem[] = {0, (uint8_t)strlen(str_value)};


The explicit cast to uint8_t could silently truncate if strlen returns a value larger than 255. While this is safe for the current test string "10m/s2" which is only 6 characters, consider adding a comment or assertion to document this size constraint, or using a safer pattern that validates the length before casting.

Copilot · 2026-02-19T09:29:21Z

-                test-udp test-commonheader)
+                test-udp test-commonheader test-commonheader-cpp
+                test-vss-cpp test-tscf-cpp test-ntscf-cpp test-flexray-cpp test-lin-cpp
+                test-most-cpp test-gpc-cpp test-sensor-cpp test-udp-cpp 


There is a trailing space at the end of this line that should be removed for consistency and to avoid potential linting issues.

Suggested change

test-most-cpp test-gpc-cpp test-sensor-cpp test-udp-cpp

test-most-cpp test-gpc-cpp test-sensor-cpp test-udp-cpp

Copilot · 2026-02-19T09:29:21Z

+    VssData_t vss_data_get = {0};
+    VssDataInt64Array_t vss_data_int64_arr_get = {0};
+    vss_data_int64_arr_get.data = 0;
+


There is an inconsistent extra blank line here. Other similar code blocks in this file (e.g., lines 550-553, 630-633) don't have a blank line between the struct initialization and its usage. Remove this blank line for consistency.

Suggested change

Copilot · 2026-02-19T09:29:21Z

+    VssData_t vss_data_get = {0};
+    VssDataFloatArray_t vss_data_float_arr_get = {0};
+    vss_data_float_arr_get.data = 0;
+


There is an inconsistent extra blank line here. Other similar code blocks in this file (e.g., lines 550-553, 630-633) don't have a blank line between the struct initialization and its usage. Remove this blank line for consistency.

Suggested change

adriaan-niess · 2026-02-19T17:33:08Z

I think the additional build targets in CMakeLists.txt that have been added for C++ are redundant and hard to maintain (always need to mirror the C-based build targets). I'd suggest do modify the test_all.sh script and do a second build/test with C++ compiler instead of C compiler just by passing the right command line argument.

nayakned · 2026-02-20T15:35:23Z

I think the additional build targets in CMakeLists.txt that have been added for C++ are redundant and hard to maintain (always need to mirror the C-based build targets). I'd suggest do modify the test_all.sh script and do a second build/test with C++ compiler instead of C compiler just by passing the right command line argument.

Hi @adriaan-niess. These additional build targets would have identified the error that caused issue #115. These run the tests alone in C++ mode while keeping the library in C, which is the primary deployment scenario for Open1722 for now.

The idea of having a parameterised build to choose the compiler is elegant. However, chasing this avenue turned out to be far from trivial with the already convoluted CMake Scripts Open1722 has. I would be happy if you can make a proposal.

Signed-off-by: Naresh Nayak <Naresh.Nayak@de.bosch.com>

nayakned · 2026-02-20T17:27:04Z

One more improvement: I added a function (with help of Copilot) which will automatically generate the CPP test target. So there is no need of a manual "mirroring" of C-based test targets.

SebastianSchildt

lgtm 🍏

nayakned added 2 commits February 19, 2026 05:39

Moved unit tests to CXX files to test C++ bindings implicitly.

ec52da5

Signed-off-by: Naresh Nayak <Naresh.Nayak@de.bosch.com>

Fixed SPDX license headers for unit test files.

db25d7f

Signed-off-by: Naresh Nayak <Naresh.Nayak@de.bosch.com>

nayakned force-pushed the test/cpp_bindings branch from 67c7bd0 to db25d7f Compare February 19, 2026 05:45

Unit test run with C as well as C++ compiler to check for compilation.

172ee9e

Signed-off-by: Naresh Nayak <Naresh.Nayak@de.bosch.com>

nayakned requested review from SebastianSchildt and Copilot February 19, 2026 09:24

Copilot started reviewing on behalf of nayakned February 19, 2026 09:25 View session

Copilot AI reviewed Feb 19, 2026

View reviewed changes

nayakned requested a review from adriaan-niess February 19, 2026 16:31

Replaced duplicate CPP target creation using a function.

6650479

Signed-off-by: Naresh Nayak <Naresh.Nayak@de.bosch.com>

SebastianSchildt reviewed Feb 22, 2026

View reviewed changes

SebastianSchildt approved these changes Feb 22, 2026

View reviewed changes

SebastianSchildt merged commit c859207 into COVESA:main Feb 22, 2026
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moved unit tests to CXX files to test C++ bindings implicitly.#117

Moved unit tests to CXX files to test C++ bindings implicitly.#117
SebastianSchildt merged 4 commits intoCOVESA:mainfrom
nayakned:test/cpp_bindings

nayakned commented Feb 19, 2026

Uh oh!

SebastianSchildt commented Feb 19, 2026

Uh oh!

nayakned commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

Copilot AI Feb 19, 2026

Uh oh!

adriaan-niess commented Feb 19, 2026 •

edited

Loading

Uh oh!

nayakned commented Feb 20, 2026

Uh oh!

nayakned commented Feb 20, 2026

Uh oh!

SebastianSchildt left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	test-most-cpp test-gpc-cpp test-sensor-cpp test-udp-cpp
	test-most-cpp test-gpc-cpp test-sensor-cpp test-udp-cpp

Conversation

nayakned commented Feb 19, 2026

Uh oh!

SebastianSchildt commented Feb 19, 2026

Uh oh!

nayakned commented Feb 19, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 19, 2026

Choose a reason for hiding this comment

Uh oh!

adriaan-niess commented Feb 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nayakned commented Feb 20, 2026

Uh oh!

nayakned commented Feb 20, 2026

Uh oh!

SebastianSchildt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

adriaan-niess commented Feb 19, 2026 •

edited

Loading