Skip to content

Opencmw-cpp interoperability fixes and testing #239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wirew0rm wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Opencmw-cpp interoperability fixes and testing #239

wirew0rm wants to merge 4 commits into from

Conversation

@wirew0rm
Copy link
Member

@wirew0rm wirew0rm commented Dec 19, 2025 •
edited
Loading

Follow-up of #223 and companion PR to fair-acc/opencmw-cpp#389
Should be merged after that one, to ensure that the compatibility test is correctly run.

Changes:

  • metadata: remove direction and groups metadata fields and add modifier field, to match opencmw-cpp
  • add quantity metadata field
  • serialise char as 8bit wide character as opposed to java, which serialises it as a 16bit wide character
  • fetch a simple test binary from the opencmw-cpp repo and use it to verify cross compatibility

missing functionality:

  • fetch test binary from opencmw-cpp release artifacts
  • fix serialiser test failure (probably an errror in the test implementation since the deserialisation of subscription updates works)
  • find out why tests fail/flake in the CI. (they run fine locally and do not really seem to be directly connected)

github-advanced-security[bot]
github-advanced-security bot found potential problems Dec 19, 2025
View reviewed changes
@wirew0rm wirew0rm temporarily deployed to configure coverage January 5, 2026 16:33 — with GitHub Actions Inactive
@wirew0rm wirew0rm had a problem deploying to configure coverage January 5, 2026 16:33 — with GitHub Actions Failure
github-advanced-security[bot]
github-advanced-security bot found potential problems Jan 5, 2026
View reviewed changes
@wirew0rm
BinarySerialiser: Update Metadata to sync with cpp
bdb3a9e 
Opencmw-java and opencmw-cpp used different metadata entries for the
field descriptions. While opencmw-cpp serialised [description(string),
unit(string), modifier(byte/enum)], opencmw-java used
[description(string), unit(string), direction(string),
groups(string[])].
This lead to failures in deserialising c++ serialised field medatadata.

These changes make opencmw-java use the same format as c++.

Signed-off-by: Alexander Krimm <[email protected]>
@wirew0rm
BinarySerialiser: add quantity metadata field
1dc6380 
Signed-off-by: Alexander Krimm <[email protected]>
@wirew0rm
Serialise char as single byte like c++
14873a2 
char in c++ is usually represented as 1 byte, while java chose 2 bytes.
To allow for interoperability, this changes the serialisation to only
use a single byte.

Since this replaces a memcpy by a naive for loop, it could probably be
improved by doing it in a tight loop, and using more efficient
conversion functions. This first step gets the 2 implementations to be
consistent, performance improvements can be added later on.

Signed-off-by: Alexander Krimm <[email protected]>
@wirew0rm wirew0rm had a problem deploying to configure coverage January 5, 2026 16:50 — with GitHub Actions Failure
@wirew0rm
Add a test to ensure interoperability with the c++ version
1a14a65 
To run the tests locally, copy the Compatibility test executable from
opencmw-cpp to the repository root.
The CI will fetch it from the artifact of opencmw-cpp.
If it is not available, the tests will be skipped.

Signed-off-by: Alexander Krimm <[email protected]>
@wirew0rm wirew0rm had a problem deploying to configure coverage January 5, 2026 16:58 — with GitHub Actions Failure
@wirew0rm wirew0rm had a problem deploying to configure coverage January 5, 2026 16:58 — with GitHub Actions Failure
@wirew0rm wirew0rm had a problem deploying to configure coverage January 5, 2026 17:14 — with GitHub Actions Failure
@wirew0rm wirew0rm had a problem deploying to configure coverage January 5, 2026 17:14 — with GitHub Actions Failure
@wirew0rm wirew0rm temporarily deployed to configure coverage January 5, 2026 17:19 — with GitHub Actions Inactive
@wirew0rm wirew0rm deployed to configure coverage January 5, 2026 17:19 — with GitHub Actions Active
@wirew0rm wirew0rm marked this pull request as ready for review January 6, 2026 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@wirew0rm