Skip to content

Conversation

@marcschier
Copy link
Collaborator

@marcschier marcschier commented Aug 22, 2025

Proposed changes

Describe the changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.

Related Issues

  • Fixes #

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply. You can also fill these out after creating the PR.

  • Bugfix (non-breaking change which fixes an issue)
  • Enhancement (non-breaking change which adds functionality)
  • Test enhancement (non-breaking change to increase test coverage)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected, requires version increase of Nuget packages)
  • Documentation Update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc.
  • I have signed the CLA.
  • I ran tests locally with my changes, all passed.
  • I fixed all failing tests in the CI pipelines.
  • I fixed all introduced issues with CodeQL and LGTM.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added necessary documentation (if appropriate).
  • Any dependent changes have been merged and published in downstream modules.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

@codecov
Copy link

codecov bot commented Aug 22, 2025

Codecov Report

❌ Patch coverage is 48.73239% with 364 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.01%. Comparing base (b6b63c9) to head (7fabfa6).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...braries/Opc.Ua.Client/Subscription/Subscription.cs 65.59% 68 Missing and 7 partials ⚠️
Libraries/Opc.Ua.Client/NodeCache/NodeCache.cs 64.28% 58 Missing and 12 partials ⚠️
...braries/Opc.Ua.Client/NodeCache/IAsyncTypeTable.cs 9.67% 56 Missing ⚠️
...braries/Opc.Ua.Client/NodeCache/IAsyncNodeTable.cs 0.00% 30 Missing ⚠️
...braries/Opc.Ua.Client/Session/SessionExtensions.cs 30.23% 29 Missing and 1 partial ⚠️
Libraries/Opc.Ua.Client/Browser.cs 14.70% 28 Missing and 1 partial ⚠️
...ibraries/Opc.Ua.Client/Session/TraceableSession.cs 56.81% 19 Missing ⚠️
Libraries/Opc.Ua.Client/Session/SessionObsolete.cs 0.00% 14 Missing ⚠️
Libraries/Opc.Ua.Server/Server/StandardServer.cs 0.00% 12 Missing ⚠️
Libraries/Opc.Ua.Client/CoreClientUtils.cs 52.63% 9 Missing ⚠️
... and 4 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3174      +/-   ##
==========================================
+ Coverage   57.88%   58.01%   +0.13%     
==========================================
  Files         353      354       +1     
  Lines       80549    77908    -2641     
  Branches    14037    13816     -221     
==========================================
- Hits        46628    45201    -1427     
+ Misses      29614    28471    -1143     
+ Partials     4307     4236      -71     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

@marcschier marcschier marked this pull request as ready for review August 26, 2025 13:53
@marcschier marcschier requested a review from Copilot August 26, 2025 13:54
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR makes the synchronous client API obsolete by adding [Obsolete] attributes to sync methods and converting all test code to use the async equivalents. The changes ensure all client operations use async patterns while maintaining backward compatibility through obsolete sync methods that internally call the async versions.

  • Marks sync client methods as obsolete and implements them as wrappers around async methods
  • Converts all test code from sync to async implementations
  • Removes conditional compilation directives (CLIENT_ASYNC, NET_STANDARD_ASYNC) as async is now the default
  • Updates project files to use the NET_STANDARD_OBSOLETE_SYNC define

Reviewed Changes

Copilot reviewed 51 out of 59 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Tests/Opc.Ua.Client.Tests/SessionClientBatchTest.cs Removes sync test methods, keeps only async versions
Tests/Opc.Ua.Client.Tests/ReverseConnectTest.cs Converts sync method calls to async equivalents
Tests/Opc.Ua.Client.Tests/RequestHeaderTest.cs Updates test method to use async ReadValuesAsync
Tests/Opc.Ua.Client.Tests/Opc.Ua.Client.Tests.csproj Removes CLIENT_ASYNC define constant
Tests/Opc.Ua.Client.Tests/NodeCacheAsyncTest.cs Simplifies by removing sync/async branching logic
Tests/Opc.Ua.Client.Tests/DurableSubscriptionTest.cs Converts subscription operations to async patterns
Tests/Opc.Ua.Client.Tests/ContinuationPointInBatchTest.cs Updates browse operations to async methods
Tests/Opc.Ua.Client.Tests/ClientTestServices.cs Wraps sync interface methods with async calls
Tests/Opc.Ua.Client.Tests/ClientTestServerQuotas.cs Removes sync version of chunk reading test
Tests/Opc.Ua.Client.Tests/ClientTestNoSecurity.cs Converts discovery and session operations to async
Tests/Opc.Ua.Client.Tests/ClientTestFramework.cs Updates operation limit reading to async
Tests/Opc.Ua.Client.Tests/ClientTest.cs Extensive conversion of sync session operations to async
Tests/Opc.Ua.Client.Tests/ClientFixture.cs Updates endpoint selection to async
Tests/Opc.Ua.Client.ComplexTypes.Tests/TypeSystemClientTest.cs Converts node cache operations to async
Tests/Opc.Ua.Client.ComplexTypes.Tests/NodeCacheResolverTests.cs Updates browser operations to async
Tests/Opc.Ua.Client.ComplexTypes.Tests/DataDictionaryTests.cs Converts dictionary loading to async
Stack/Opc.Ua.Core/Stack/Configuration/ConfiguredEndpoints.cs Marks sync UpdateFromServer methods as obsolete
Stack/Opc.Ua.Core/Stack/Client/SessionClient.cs Minor code formatting change
Stack/Opc.Ua.Core/Stack/Client/IClientBase.cs Removes sync Close method from interface
Stack/Opc.Ua.Core/Stack/Client/DiscoveryClient.cs Marks sync discovery methods as obsolete
Stack/Opc.Ua.Core/Stack/Client/ClientBase.cs Removes sync Close implementation
Stack/Opc.Ua.Core/Security/Certificates/CertificateTrustList.cs Adds cancellation token to async method
Stack/Opc.Ua.Core/Opc.Ua.Core.csproj Adds NET_STANDARD_OBSOLETE_SYNC define
Libraries/Opc.Ua.Server/Server/StandardServer.cs Converts discovery registration to async
Libraries/Opc.Ua.Server/NodeManager/MasterNodeManager.cs Updates method call handling to async
Libraries/Opc.Ua.Gds.Client.Common/ServerPushConfigurationClient.cs Updates endpoint selection to async
Libraries/Opc.Ua.Gds.Client.Common/GlobalDiscoveryServerClient.cs Updates endpoint selection to async
Libraries/Opc.Ua.Client/Subscription/SubscriptionObsolete.cs New file containing obsolete wrappers for sync methods
Libraries/Opc.Ua.Client/Subscription/SubscriptionAsync.cs Removed file (async methods moved to main class)
Libraries/Opc.Ua.Client/Subscription/Subscription.cs Merges async methods into main class, removes sync implementations
Libraries/Opc.Ua.Client/Subscription/MonitoredItem.cs Converts GetEventType to async

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@romanett romanett self-requested a review August 26, 2025 18:52
Copy link
Contributor

@romanett romanett left a comment

Choose a reason for hiding this comment

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

Great addtion to streamline our codebase. Also like that the deprecation of sync & APM based generated code is already prepared / done

@marcschier marcschier merged commit 25ba2a2 into master Aug 27, 2025
119 of 120 checks passed
@marcschier marcschier deleted the asyncclient branch August 27, 2025 13:05
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