Skip to content

Fix empty LocalizedText binary field encoding - #4709

Merged
marcschier merged 8 commits into
masterfrom
fix/binary-encoder-empty-locale
Oct 10, 2026
Merged

marcschier merged 8 commits into
masterfrom
fix/binary-encoder-empty-locale

Conversation

@cristipogacean

@cristipogacean cristipogacean commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

Omit empty Locale and Text fields from the OPC UA Binary DataEncoding for LocalizedText, as required by OPC 10000-6, Section 5.2.2.14, Table 24. Add wire-format tests for empty and non-empty fields, including translation-backed localized text.

Validation:

  • git diff --check passed before commit.
  • dotnet build tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj --framework net10.0 --no-restore --verbosity minimal -p:NuGetAudit=false could not complete because access to https://api.nuget.org/v3/index.json failed with NU1301 and a TLS handshake error.

Related Issues

No related issue.

Checklist

  • I have signed the CLA and read the CONTRIBUTING doc.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added all necessary documentation.
  • I have verified that my changes do not introduce (new) build or analyzer warnings.
  • I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed.
  • I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings.
  • I have addressed all PR feedback received.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings October 9, 2026 09:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Three nullable buffer assignments trigger warnings in the warnings-as-errors build.

1 open finding
What changed in this PR

Updates binary encoding to omit empty LocalizedText locale/text fields per OPC UA requirements.

Changes:

  • Uses encoding-mask bits only for non-empty fields.
  • Adds wire-format and round-trip tests.
File Description
src/​Opc.Ua.Types/​Encoders/​BinaryEncoder.cs Omits empty localized-text fields.
tests/​Opc.Ua.Types.Tests/​BuiltIn/​LocalizedTextTests.cs Adds binary encoding tests.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/Opc.Ua.Types.Tests/BuiltIn/LocalizedTextTests.cs Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@cristipogacean cristipogacean self-assigned this Oct 9, 2026
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Code coverage

✅ Coverage gate passed.

Check Result Threshold
✅ Project line rate 88.61% (306553/345957 lines) >= 70.00%
✅ Project branch rate 79.06% >= 60.00%
✅ Patch coverage 100.00% (11/11 changed lines) >= 60.00% (<= 100 changed lines, advisory)
ℹ️ Baseline delta (advisory) +15.01 pp 73.60% recorded

Coverage is above the recorded baseline - consider ratcheting coverage-thresholds.json.

Thresholds live in coverage-thresholds.json. Whole report before exclusions: line 87.74%, branch 78.26%.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@marcschier marcschier left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to fix the build errors on AOT

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@codecov

codecov Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.90909% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 82.84%. Comparing base (2cdf0de) to head (7578a15).

Files with missing lines Patch % Lines
src/Opc.Ua.Types/BuiltIn/LocalizedText.cs 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4709      +/-   ##
==========================================
- Coverage   82.85%   82.84%   -0.01%     
==========================================
  Files        2267     2267              
  Lines      345956   345957       +1     
  Branches    60309    60311       +2     
==========================================
- Hits       286640   286625      -15     
- Misses      39219    39238      +19     
+ Partials    20097    20094       -3     
Flag Coverage Δ
actions 82.84% <90.90%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/Opc.Ua.Types/BuiltIn/LocalizedText.cs 84.85% <90.90%> (-0.30%) ⬇️

... and 33 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread tests/Opc.Ua.Types.Tests/BuiltIn/VariantStorageTests.cs Outdated
Comment thread src/Opc.Ua.Types/Encoders/BinaryEncoder.cs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Encoding-only normalization breaks LocalizedText equality and round-trip consistency.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/Opc.Ua.Types/Encoders/BinaryEncoder.cs Outdated
Normalize plain locale and text inputs and exposed formatted values while preserving translation templates and fallback metadata. Restore ordinary codec round-trip equality and cover null, empty, and whitespace fields.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cristipogacean and others added 2 commits October 9, 2026 22:00
Keep empty-field normalization in LocalizedText and restore the original Locale and Text null checks in BinaryEncoder. Adapt round-trip expectations to canonical values and cover empty and formatted translation-backed fields.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants