15 Commits

Include renames

Author	SHA1	Message	Date
3 people	5899354441	Clarify documentation, migration guidance, and SDK selection (#4823) ... # Description Remove stock phrasing and correct misleading or inconsistent technical documentation. The initial two passes covered 367 Markdown files, followed by an interactive review of 169 editorial flags. The U5 interpretation originally deferred in that review has since received the upstream Mantis answer recorded by #4843; remaining server/spec follow-ups stay explicit in the CTT log. - Correct package responsibilities, migration steps, API names, ownership, protocol descriptions and links. Keep bundled migration guides synchronized. Preserve historical conclusions as historical records with separate re-check notes. - Provide at least two rendered TOC levels throughout `docs/`. Convert appropriate architecture figures to Mermaid without changing code examples, transcripts or historical quotations. - Address the October 19 feedback with concrete setup/migration instructions, corrected certificate paths and PubSub names, shorter repeated caveats, consistent XML comments, and a full review of the Concepts guide. Retain #4839's API corrections. - Integrate #4841's runtime/performance guidance and review incoming/recent prose. Preserve dated measurements rather than claiming new benchmark runs; clarify allocation ratios, startup-only processor settings, JIT warm-up and the benchmark commit's scope. Missing raw measurement artifacts and repeat ranges remain tracked by #4842. - Merge master through `e8c5d800a` in `32c6394b7`. Resolve the CTT log conflict using the updated Mantis states and OPC answers, retaining historical framing and re-checks for C28/C32/C43. - Address the latest naming/index feedback in `adb5bf693`: rename the guides to `docs/WotCon.md` and `docs/WotNodeSetConversion.md`; move the provider and DataType-declaration contracts into the conversion guide and remove `docs/WoT.md`. Rename `samples/DI` to `samples/Di` and `samples/Isa95` to `samples/ISA95`. Add content-index READMEs to Di, FluentApi, ISA95, Lds and PubSub; update solution entries, project references, Docker/CI paths, test fixture paths and documentation links with exact casing. **WoT guide scope.** This branch retains the detailed conversion guide that #4758 had replaced with an anchor stub. Conversion limits, verified document sets, preservation rules and provider ownership now live together in `WotNodeSetConversion.md`; binding/runtime guidance remains in `WotBindings.md` and `WotCon.md`. The design-rationale/test-ledger prose was shortened, the oversized argument-table cell moved into its section, and existing anchors preserved. **SDK policy is a separate, explicit user-directed change.** `global.json` sets a stable minimum of `10.0.400`, `rollForward: latestMajor`, and `allowPrerelease: false`. Newer major SDKs are permitted; CI's explicit SDK pins are unchanged. README, GettingStarted, DeveloperGuide and Recycling instructions reflect the possible analyzer differences. The image-contract regression fix accepts compatible stable SDK images instead of requiring exact equality with the floor. The latest four rename/index threads are resolved after publication. Three older threads remain open: universal obsolete-removal timing, unconditional nonce-strength wording, and a blanket Vision NativeAOT claim. Existing implementation/evidence constraints are explained in their inline replies. The two-level TOCs remain as requested. ## Validation - `git diff --check` and staged diff checks: passed. The untouched upstream AutoID NodeSet retains its provenance bytes, including two existing license-header trailing spaces. - `.\.azurepipelines\validate-migration-plugin.ps1`: passed; 15 bundled migration documents and manifests synchronized. - Documentation/path validation: **101** documents retain two rendered TOC levels (**2,228** entries), with idempotent formatting. **52** new relative links and their anchors passed exact-case checks. No tracked references to the old guide/sample paths remain. Both relocated provider-contract sections are preserved in the destination. - `dotnet sln UA.slnx list`: passed; shows the renamed Di/ISA95 projects and the existing FluentApi/Lds/PubSub projects. Updated solution and pump ProjectReference paths also passed exact-case existence checks. - Latest focused commands, run from the repository root: ```powershell # Passed: 61 channel send-order and reconnect tests. dotnet test tests\Opc.Ua.Core.Tests\Opc.Ua.Core.Tests.csproj -p:CustomTestTarget=net10.0 --filter 'FullyQualifiedName~UaSCBinaryChannelSendOrderTests|FullyQualifiedName~TcpReconnectKeyAgreementTests' --verbosity minimal # Passed: 5 pump datasheet tests; builds the renamed pump sample. dotnet test tests\Opc.Ua.Di.Tests\Opc.Ua.Di.Tests.csproj -p:CustomTestTarget=net10.0 --filter 'FullyQualifiedName~PumpDatasheetConformanceTests' --verbosity minimal # Passed: 4 repository NodeSet path/round-trip tests, including the moved pump models. dotnet test tests\Opc.Ua.Types.Tests\Opc.Ua.Types.Tests.csproj -p:CustomTestTarget=net10.0 --filter 'FullyQualifiedName~RepositorySourceSurvivesSerializedLinkedPartitionsAsync' --verbosity minimal # Passed: build, zero warnings/errors. dotnet build samples\ISA95\MinimalIsa95Server\MinimalIsa95Server.csproj -p:CustomTestTarget=net10.0 -f net10.0 --verbosity minimal # Passed: dependent sample-test project build, zero warnings/errors. dotnet build tests\Opc.Ua.WotCon.Samples.Tests\Opc.Ua.WotCon.Samples.Tests.csproj -p:CustomTestTarget=net10.0 -f net10.0 --verbosity minimal ``` Earlier focused integration checks passed 2,620 tests across AutoID, event queues/scheduling, source generation, MCP/package profiles and WoT. Both reviewed Mermaid diagrams rendered locally without network access. These checks are not a full solution run. The full solution has **not** been rerun for this review. Earlier `dotnet test UA.slnx -p:CustomTestTarget=net10.0 --verbosity quiet` did not pass: three AOT projects rejected VSTest and 20 Vision tests failed during graphics initialization with `OutOfMemoryException`. Remaining long-running tests were stopped at the user's request. A focused Vision rerun passed 558 tests and reproduced those 20 failures. The earlier `dotnet test UA.slnx -p:CustomTestTarget=net48 --verbosity quiet` was blocked by the former SDK pin and was not retried after the policy change. The user approved proceeding with those limitations documented; current remote CI is not claimed green. ## Related Issues - Follow-up #4842 tracks the requested 2.0.2+ benchmark/tuning and diagnostic-tool documentation refresh. #4841 supplies documentation and earlier results, not all newly requested measurements. - #4758 explains the conversion guide's earlier consolidation; the current provider contracts are now consolidated into `WotNodeSetConversion.md`. - #4839 supplies the reviewed API corrections; #4843 supplies the updated CTT issue record. - The original prose cleanup and SDK policy were user-requested without a source issue. No new runtime architecture is introduced by these documentation edits. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] 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: Roman Ettlinger <romanett98@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-21 10:10:55 +02:00
Marc Schier	2836da7ec8	Fix sessionless ResultTransfer ownership bypass and other issues. (#4834) ... # Description Correct ownership, endpoint validation, policy propagation, subscription delivery, file handling and generated-model behavior across the stack, tools and samples. Add focused regression tests and update the corresponding guides and sample documentation. ### Previously included corrections - Require an active, matching session to generate or access transient ResultTransfer file handles. Cover both sessionless generation and reads against a handle owned by an active session, using live session owners in the Machinery tests. - Keep physical filesystem operations within their configured mount and apply the same confinement checks throughout file operations. - Validate LDS semaphore paths consistently during server registration and registration-store maintenance. ### Individual issues addressed | # | Component | Issue and correction | |---|---|---| | 1 | AI jobs and transfers | Jobs and transfers did not consistently retain their creator's identity and session. Apply ownership checks to descendants, file handles, subscriptions and execution, and recheck deployment permissions before dispatch. | | 2 | Password-management client | Password-bearing calls could use signing-only channels or follow a replacement connection. Capture an encrypted transport generation and keep dispatch bound to it; preserve ordinary non-password edits. | | 3 | GDS and certificate-push clients | Connections omitted hostname verification. Validate the original configured DNS name or IP address before session activation. | | 4 | Certificate push | Private-key uploads could use signing-only calls. Require an encrypted, generation-bound dispatch while preserving certificate-only updates and restoring the caller's identity afterward. | | 5 | MQTT client | The .NET Framework custom-trust callback could discard TLS hostname, certificate-availability and usage errors. Retain those checks while supporting configured issuer trust. | | 6 | DTLS handshake | Certificate trust was not tied to the configured peer address. Match the certificate's DNS or IP identity to the intended endpoint. | | 7 | Replicated address spaces | Serialized nodes lost per-node and descendant permission metadata. Preserve role permissions, user-role permissions and access restrictions, including updates to existing replica nodes. | | 8 | MCP PubSub imports | Imports without groups skipped checks on the effective broker destination. Check the actual endpoint before resolving credentials or activating the connection. | | 9 | Reference-server accounts | A disabled username account could authenticate again with its previous roles. Reject disabled accounts during fresh authentication. | | 10 | Console reference client | The sample disabled hostname checks when connecting to an otherwise trusted peer. Retain the original requested host and validate it before activation. | | 11 | Capture replay and export | Replay and raw export could accept non-capture files or include key material. Require complete PCAP/PCAPNG containers and exclude key sidecars and decryption-secret blocks. | | 12 | PubSub adapter fallback | The fallback client configuration automatically accepted peer certificates. Apply normal trust and hostname validation before activation or reads. | | 13 | Kafka transport | Explicit TLS enablement was checked but did not reach both final Confluent configurations. Use the same effective protocol for validation, producers and consumers. | | 14 | PubSub actions | Configuration rebuilds could widen a connection-local action handler's scope. Preserve its original connection association and reject mismatched dispatch. | | 15 | PubSub readers | Reader-level message-protection settings were accepted but ignored. Apply the effective group and reader settings before accepting and publishing received values. | | 16 | PubSub key groups | Different `SecurityGroupId` values could share one connection-wide key context. Reject mixed groups on a connection and require separate connections for distinct groups. | | 17 | MCP session registrations | Session-name replacement or reuse could retain another registration's credential bindings. Bind access to the registration, caller and authenticated endpoint, and preserve that identity when borrowing sessions. | | 18 | Viewer HTTP host | Requests reached the viewer's OPC UA session without consistent Host, Origin and port checks. Validate the request destination and browser origin before using the session, while retaining supported local MCP clients. | | 19 | Vision file operations | File operations did not consistently apply protected-file checks, and remote handles lacked caller and connection context. Apply the shared checks and use host-side opaque handles bound to caller, target and generation. | | 20 | DTLS record integrity | The integrity-only MAC used a truncated sequence number. Authenticate the full reconstructed sequence number and epoch so records remain distinct across rollover. | | 21 | AMB client sample | Connection setup omitted hostname validation. Preserve the originally requested host and verify the peer identity before activation. | | 22 | Robot sample writes | The R2 `SpeedOverride` callback could target R1 state. Bind each callback to its own robot and apply that robot's write policy. | | 23 | AI result properties | Optional result children inherited remotely writable defaults. Preserve their declaration restrictions and keep server-produced results read-only. | | 24 | WSS peer-trust updates | Existing channels did not consistently respond to peer-trust changes. Revalidate complete chains for forward and reverse WSS channels and close only those that no longer validate. | | 25 | GDS revocation checks | Caller-provided certificates could cause issuer retrieval from embedded URLs. Resolve validation from configured local trust, issuers and CRLs without fetching caller-provided locations. | | 26 | GMS observations | Published correction observations retained writable model defaults. Reject remote writes while preserving publisher-side updates. | | 27 | Production-dataset monitoring | Subscription admission and delivery could use the reporter's context instead of each subscriber's permissions. Evaluate the subscriber at admission and on every delivery, including cancellation and disposal paths. | | 28 | WoT MQTT destinations | Hostless MQTT forms could imply a localhost destination before loopback checks. Reject them before credential lookup or connection attempts and validate the effective endpoint. | | 29 | XRegistry opaque reads | Content-ID reads and notifications did not consistently honor encrypted-read requirements or the currently accepted transport. Recheck the bound endpoint and reject transport downgrades before rebinding, including when protected values are already queued. | | 30 | PubSub capture dissection | Key loading did not consistently check the configured capture-root boundary. Check each actual key path immediately before opening it. | | 31 | PubSub publication queues | Imported WriterGroup queues were not fully checked against publish-topic grants. Check the destination actually used by each group. | | 32 | MCP PubSub mutations | A caller-supplied mode could select subscribe grants for an operation that publishes. Derive the required direction and parent context from the actual mutation. | | 33 | Generated write masks | Model import and generation could widen an explicit `UserWriteMask`. Preserve it through import, copy, merge, inheritance and dependency metadata. | | 34 | Generated method flags | An explicit `UserExecutable=false` could be lost. Preserve its specified state and resulting method behavior through the same model paths. | | 35 | Generated role identifiers | Unresolved role identifiers could cause permission entries to be omitted. Preserve namespace-qualified role IDs and report a diagnostic when a policy cannot be represented. | | 36 | Aggregation-server defaults | The `localhost` default did not reliably constrain listener binding. Use numeric loopback for the default endpoint and command-line fallback. | | 37 | Certificate-validation cache | Cached results could skip current leaf or issuer validity checks. Recheck validity windows on cache hits and keep hostname validation independent of cached chain trust. | | 38 | Software-update folders | Revision names and marker contents containing dot segments could select paths outside the software root. Normalize and check every derived path against that root. | | 39 | USDA arrays | Array elements were not encoded for their string or asset context. Escape them as data and preserve valid composition and round-trip behavior. | | 40 | Adapter hot reload | A changed dataset definition with the same name could keep old fields, subscriptions or cached values active. Compare complete immutable definitions and retire old state before replacement. | | 41 | Adapter index ranges | Subscription publication could ignore `IndexRange` and return the complete value. Preserve the requested projection and its status and timestamps; reject unsupported range providers. | | 42 | PubSub metadata and targets | Metadata from one connection could change another connection's target mapping. Scope metadata to its connection and provenance, and resolve targets using actual typed field IDs rather than field names or unintended positional fallback. | | 43 | Scales product locks | Product locks were not applied across every remote mutation path. Apply them consistently to value writes, filling calculations and related recipe/controller operations while retaining permitted updates. | | 44 | Event permission caching | Cached event decisions could outlive identity, role or node-policy changes. Revalidate the current subscriber, event type and source permissions instead of reusing a stale decision. | | 45 | WoT Modbus destinations | Hostless Modbus forms could imply localhost before loopback checks. Reject them before connecting and validate the effective endpoint. | | 46 | MCP local PKI tools | Peer-trust mutations did not consistently require the caller's `CertificateAdministration` grant. Check the capability before trust and untrust operations. | | 47 | Robot-control HTTP bridge | The bridge lacked consistent Host, Origin and port validation and caller-specific session grants. Apply those checks before borrowing the OPC UA session and retain supported local clients. | | 48 | Subscription transfers | Transferred subscriptions could retain cached notifications after the destination's permissions changed. Require authorization continuity between source and destination identities and roles, invalidate stale retransmission data when continuity fails, and preserve retransmission behavior for compatible transfers. | ### Compatibility notes - Replication uses the versioned NSRP v1 policy envelope. Upgrade peers together and re-encode or reseed legacy topology from the authoritative model; missing permission metadata is not inferred from old records. - Integrity-only DTLS peers must be upgraded together for the full sequence/epoch MAC. AEAD behavior is unchanged. - Distinct PubSub key groups require separate connections. - Custom sessions that cannot capture the actual encrypted transport, and subscription providers that cannot honor a requested range, are rejected rather than silently changing the requested behavior. - Subscription transfers retain retransmission data only when the destination authorization remains compatible with the source snapshot; otherwise the old sequence numbers and cached data are invalidated. ## Related Issues No separate tracking issue is linked. The individual corrections are listed above. ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] 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. - [x] 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.	2026-10-21 05:37:29 +02:00
romanettandClaude Opus 5.5	9b034f8291	Enable nullable reference types centrally; migrate all unset and annotations-only projects (#4700) ... # Description Nullable reference types are now enabled once, in `common.props`, and every project in the repository builds warning-clean with them. **Configuration** - `<Nullable>enable</Nullable>` lives in `common.props`. All per-project `<Nullable>` settings are removed: 178 project files (enable, annotations, and the `disable` in the three Aot test projects). - 419 per-file `#nullable enable`/`restore` directives and the 4 `#nullable disable` directives are removed. - Kept on purpose: - The 5 `<auto-generated>` schema files (`UANodeSet.cs`, `UA Type Dictionary.cs`, `OPCBinarySchema.cs`, `SecuredApplication.cs`, `PInvokeHelper.cs`) keep `#nullable enable`. The compiler ignores the project setting in generated code, so the directive is required there. - `tests/Opc.Ua.Interop.LegacyPeer` keeps `disable`. It builds against the 1.5.378 packages and does not import the repository props. - `docs/DeveloperGuide.md` and `.github/copilot-instructions.md` now describe the central setting. **Migration** (the 43 unset and 13 annotations-only projects, roughly 10k diagnostics) - **Production code: annotated at the declarations.** Covers `tools/Opc.Ua.SourceGeneration.Core`, `src/Opc.Ua.Lds.Server` and the LDS sample. - Members, parameters and returns are nullable where null actually flows. - Null-check helpers carry `[NotNullWhen]` (`IsNull`, `IsValidXmlName`, `IsMethodTypeDesign`). - `CoreUtils.Format`, `Utils.Format`, `ITemplateWriter.WriteLine` take `params object?[]`. `Template.AddReplacement` takes `string?` (rendering already skips null replacements). - `IRegisteredServerStore.RegisterAsync` returns `Task<RegistrationEntry?>`: it returns null when a registration is removed. - Dereferences backed by an invariant (for example, the validated model) use `!`, so runtime behaviour is unchanged. - **Behavioural changes in production code** (small, deliberate): - `NodeSetToModelDesign` throws `InvalidOperationException` when the file is not a NodeSet. Before, it threw a NullReferenceException on the next line. - `ModelDesignValidator` starts its lookup tables empty instead of null. A redundant `if (m_nodes == null) return;` guard is gone. - **Test code: minimal annotations.** - `?` on values that can be null, `!` where the test asserts non-null. - `= null!` for fixture members assigned in SetUp. - `TryGetStructure<T>(out T? x)` where the inferred `T?` broke the `IEncodeable` constraint. - A few redundant `== null` checks on structs (`NodeId`, `ArrayOf<T>`) folded into the adjacent `IsNull`/`Count` checks. **Notes for reviewers** - The diff is large (~1,500 files) but almost entirely mechanical: `?`, `!` and directive removal. The non-annotation changes are listed above. - Known follow-up, not done here: `IModelDesign.Namespaces/TargetNamespace/NamespaceUris` stay non-nullable even though the validator returns null before it loads a dictionary. Making them nullable would ripple through every generator call site. ## Related Issues - Fixes #4439 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] 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. The "no new warnings" box covers net10.0 and net48: full non-incremental builds of `UA.slnx`, `tools/SourceGeneration.slnx`, `tools/MigrationAnalyzer.slnx` and the Aot test projects finished with 0 warnings and 0 errors under the repository's warnings-as-errors settings. net9.0 and net8.0 also had no nullable diagnostics, but they were not rebuilt after the final fixes. Tests have not been run locally yet. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-08 08:48:13 +02:00
romanettandClaude Opus 5.5	61edaa1ed0	Remove netstandard2.1 and net472 target frameworks (#4624) ... # Description Drops support for **netstandard2.1** and **.NET Framework 4.7.2**. Libraries now target `net48`, `net8.0`, `net9.0` and `net10.0`. `Opc.Ua.Types` and `Opc.Ua.SourceGeneration.Core` still also target `netstandard2.0`, but only because the Roslyn source generators load them. There is no general netstandard2.0 support. **Preprocessor directives** (112 files) - Every `#if`/`#elif` mentioning `NETSTANDARD2_1*`, `NET472*`, `NET_STANDARD_TESTS`, `SKIP_ECC_CERTIFICATE_REQUEST_SIGNING` or `NETSTANDARD*` was evaluated against the remaining TFM set (net48, net8/9/10, plus netstandard2.0 inside `Opc.Ua.Types`/`tools`) and simplified. Dead branches were removed. - Example: `NETSTANDARD2_1 || NET8_0_OR_GREATER` → `NET8_0_OR_GREATER`. - Example: `NET8_0_OR_GREATER || NET472_OR_GREATER` → removed (always true). - Netstandard-only code was removed: `UnixFileModeShim`, the CRDT reflection path in `ReplicatedGossipOptions.CreateReaderOptions`, and the netstandard2.1-only test variants. **Custom test logic** - `targets.props` drops the `net472`, `netstandard2.0` and `netstandard2.1` `CustomTestTarget` profiles, `NetStandardTests` and `SKIP_ECC_CERTIFICATE_REQUEST_SIGNING`. Supported values are now `net48`, `net8.0`, `net9.0`, `net10.0`. - The `NET_STANDARD_TESTS` define is removed from 29 test projects. - The "build netstandard shells as net10.0" overrides are removed from 18 projects. - `RestrictForLegacyTfm` now only applies to `net48`. - `tests/customtest.bat` is updated to the new values. **Build / packaging / CI** - MSBuild TFM conditions are simplified: Kestrel package choice, MQTTnet v4 (net48 only), BouncyCastle, Di/OpenUsd/Vision/Stress test excludes, and `System.IO.FileSystem.AccessControl` (dropped from Core). - The MigrationAnalyzer nuspec drops its net472/netstandard2.1 lib groups. The Azure signing lists are updated. - `get-ci-matrix.ps1` removes the net472/netstandard test profiles (including the PR-gating `linux-netstandard2.1`) and the matching build legs. **Breaking API change (2.0 preview):** removed `HttpsTransportListener.IsWssTransportSupported` and `ReplicatedGossipOptions.IsGossipStateDecodingSupported`. They only returned `false` on netstandard2.1 and are now always `true`. The test guards that used them were removed as well. **Docs** - NuGet READMEs, `README.md`, `DeveloperGuide.md`, `PubSub.md`, the certificate/ECC/identity docs and the 2.0 migration guide (plus its `.agents` skill mirror) are updated. - The migration guide notes that 4.7.2 consumers must move to 4.8 and netstandard2.1 consumers to net8.0+. ### Reviewer notes - The `System.IO.FileSystem.AccessControl` `PackageVersion` pin is left in `Directory.Packages.props`. It has no direct reference any more, but transitive pinning is on, so it was kept to avoid shifting resolved versions. - The `#if` rewrite was done mechanically by a truth-table evaluator. The diff in the `.cs` files is limited to directive lines, removed dead branches, and stale-comment updates. ### Verified locally - `dotnet build UA.slnx -p:CustomTestTarget=net48`: 0 errors. - `dotnet build UA.slnx -p:CustomTestTarget=net10.0`: 0 errors. - Default multi-target builds (no `CustomTestTarget`) of Opc.Ua.Types, Bindings.Https, PubSub.Mqtt and Redundancy.Server succeeded. - net10.0 tests: - Opc.Ua.Redundancy.Server.Tests: 840/840 passed. - Opc.Ua.Types.Tests: 11363 passed, 1 skipped. - ModelDependencyScanner tests: 20/20 passed. - The full test suite was **not** run locally. ## Related Issues - None ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] 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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-02 17:09:49 +02:00
Marc SchierandCopilot Autofix powered by AI	e9313ef957	Only use GitHub action for CI and nightly, establish release-branch-only package publication (#4513) ... ## Description Two related changes to how the 2.0 line is built and shipped. ### 1. Release-branch-only package publication - `master` produces development previews only. - Stable packages can only be built from a canonical `release/<major>.<minor>` branch and are promoted manually from the exact signed candidate artifact. - Core packages use explicit `major.minor.patch` versions (`2.0.0`, `2.0.1`, then `2.1.0`); git height never changes a stable package patch. - XRegistry, WoT Connectivity, Vision, Robotics, Redundancy, Positioning, OpenUSD, ISA95, AI, DI, the relevant MCP extensions, and OpenUSD connector tools remain `-preview.N` while following the core numeric version. - nuget.org receives only non-`.Debug` package IDs; GitHub Packages retains the complete signed Release/Debug candidate. - `docs/ReleaseProcess.md` is a reproducible runbook for developers and agents, with each step stating its completion evidence. Promotion is source-bound and byte-checked: `release.yml` checks out nothing until the candidate SHA is known, and `assert-published-packages-match.ps1` proves that any version already on a feed holds *this* candidate's content before `--skip-duplicate` is allowed to skip it. Canonical release-branch naming is matched case-sensitively with no leading zeroes, and `version.json`'s `publicReleaseRefSpec`, both policy helpers and the copy `release.yml` inlines are pinned to one expression by tests. ### 2. PR CI moved to GitHub-hosted runners - `.github/workflows/buildandtest.yml` (**CI**) now owns pull-request validation outright, covering the Windows Debug/Release net48 + net10.0 solution builds and fast tests that only Azure Pipelines ran, alongside the existing Linux/macOS, all-target and NativeAOT legs. `build-and-test summary` remains the stable required check name. - `.github/workflows/nightly.yml` is a new **manual-only** (`workflow_dispatch`) full-scope workflow covering the Azure nightly workload: all solutions x seven target profiles x Debug/Release, the net472/netstandard profiles, Windows + Linux NativeAOT, and the long-running and durable tiers. - Shared deterministic discovery/batching lives in `.github/scripts/get-ci-matrix.ps1` with a local composite action, so both entry points expand the same workload manifest instead of duplicating a large embedded script. - `azure-pipelines.yml` is set to `pr: none`. - The standalone pump sample image workflow is folded into the consolidated **Images CI** workflow, which now builds all ten images and publishes an `images summary` rollup suitable as a required check. ## Related Issues No related issue provided. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] 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. - [x] I have addressed **all** PR feedback received. ## Validation - `Opc.Ua.Tools.Tests` full `net10.0` suite: **959 passed, 1 skipped, 0 failed**. - Version/package-policy regression subset (`PackageVersionPolicyTests`): **60 passed, 0 failed**. - GitHub Actions **CI** run 35719807620: **149/149 jobs green** on GitHub-hosted runners. - Canonical release-branch rule verified against the pinned nbgv 3.7.115 by building throwaway branches: `release/2.0`, `release/0.9`, `release/10.42` -> `PublicRelease=True`; `release/02.0`, `release/2.00`, `Release/2.0` -> `False`. - Package identity gate verified against a live nuget.org package: served packages carry a repository `.signature.p7s`, so equality is decided on content excluding that part. Re-signing and zip reordering produce an identical digest; a one-byte content change does not. - Synthetic Release-only, Debug-only, combined, preview, duplicate, orphaned-symbol, mismatched-root and accidental-stable-family package sets exercise the shared validator. - Stable-release rehearsal in an isolated `release/2.0` clone: `Opc.Ua.Di` packed as `2.0.0-preview.6` depending on stable Core `2.0.0`; `Opc.Ua.Di.Client` likewise against stable Client `2.0.0`. - All workflow YAML, `version.json`, the package-policy PowerShell scripts and the internal release-runbook anchors parse. ## Known validation limits - Full `UA.slnx` `net48` and `net10.0` local validation remains blocked by pre-existing `NETSDK1005` and dependency-version failures in unmodified fuzz/sample projects; the corresponding checklist items intentionally stay open. - The TRX-driven test verdict added for the intermittent at-exit host stall is proven by `CiTestVerdictTests`; the stall itself has not recurred since. - `nightly.yml` has not yet been run against a full-scope Azure run for tuple-level parity — see below. ## Operational follow-up This remains a preparation PR targeting `master`. It does **not** authorize a release. Release sequence: 1. Merge this PR. 2. Create canonical `release/2.0`, advance `master` to `2.1.0-preview.{height}`, and open a separate `2.0.0` version PR against `release/2.0`. 3. Follow [`docs/ReleaseProcess.md`](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/docs/ReleaseProcess.md) for candidate dry-run, environment approval, promotion, tag/release finalization, and next-patch preview setup. CI migration steps still needing administrator access are tracked in [`docs/DeveloperGuide.md#migration-status`](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/docs/DeveloperGuide.md#migration-status): proving `nightly.yml` parity on a trusted SHA with the private fuzz corpus provisioned, turning off definition 14's service-side PR override, retiring the Azure full-scope schedules and push trigger, and retiring the duplicate `azure-pipelines-preview.yml` publisher. > **Before cutting `release/2.0`:** the `Release` ruleset requires the Azure context for *every* `refs/heads/release/*`, but a new 2.x branch inherits `pr: none`, so that context would never report and would block every PR into the new line. Exclude the new ref from that ruleset, or add a 2.x ruleset requiring `build-and-test summary`, as part of creating the branch. --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>	2026-09-23 15:47:43 +00:00
Marc Schier	fd7c88c136	Add federated OpenUSD site demo (#4393) ... # Description Adds a one-command federated OpenUSD site demo and fixes the composition path that previously produced an empty, overlapping, or poorly framed viewport. - Add `samples/OpenUsd/SiteCompositionServer/run-site-composition-demo.ps1` to build and start the pump, generator, and site servers, wait for every endpoint, publish the connector and viewer side by side, launch the federated viewport, and clean up child processes and artifacts. - Include every fetched root layer in the generated stage and prepare structural component composition before the retained renderer starts. - Continue persisting structural changes while forwarding value-only updates into the live viewport, avoiding renderer topology invalidation. - Fold structured translation, rotation, and scale values into `xformOp:transform` matrices in `UsdFileSink`, including time samples, so multiple pump and generator instances remain distinct when the stage is reopened. - Preserve the latest prior transform components when independently sampled translation, rotation, or scale history adds a new matrix sample. - Reframe the site camera to show all three default pumps and both generator sets. - Move the detailed root sample catalog to `docs/samples.md` and replace it with a concise link to the platform-independent samples and companion sample repository. Validation performed: - Ran the demo script through both its existing-viewer-bundle path and its default side-by-side publish path on isolated ports. - Ran the focused OpenUSD connector and file-sink tests: 30 passed. - Ran the full `Opc.Ua.OpenUsd.Tests` project: 919 passed; the remaining local failure is the existing culture-sensitive decimal formatting assertion on this host's `en-DE` culture. - Ran the focused review-feedback file-sink tests: 19 passed. ## Related Issues - Fixes #4392 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] 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. - [x] I have addressed **all** PR feedback received. --------- Copilot-Session: 797ea461-49e2-4844-b6a4-54565a9469bd	2026-09-03 10:34:21 +02:00
Marc SchierandCopilot	c84c168585	Merge OpenUSD scene packages (#4384) ... # Description Merge the draft OpenUSD Scene Materialization projects into the existing OpenUSD core and server packages: - move the scene companion model, document/value model, and `.usda` conversion APIs into `Opc.Ua.OpenUsd`; - move discovery, materialization, export, and Part 1 interop into `Opc.Ua.OpenUsd.Server`; - rename the CLR APIs to `Opc.Ua.OpenUsd.Scene`, `Opc.Ua.OpenUsd.Scene.Conversion`, and `Opc.Ua.OpenUsd.Server.Scene`; - remove the `Opc.Ua.OpenUsdScene` and `Opc.Ua.OpenUsdScene.Server` projects and NuGet package IDs; - update the connector, the consolidated OpenUSD test project, package READMEs, and developer docs. This is an intentional source- and binary-breaking cleanup for the draft Scene packages. The OPC UA Scene model URI, version, NodeIds, and wire behavior remain unchanged. ## Related Issues - Fixes #4383 ## Validation - `UA.slnx` Release builds completed for `CustomTestTarget=net10.0` and `CustomTestTarget=net48`. - OpenUSD tests passed on net10.0 (915) and net48 (879), excluding the existing host-culture assertion; the isolated assertion passed under invariant globalization. - The generated Release pack contains the expected 75 packages, emits neither retired Scene package, and passes `.azurepipelines\validate-source-generator-packages.ps1`. - The two surviving packages expose the new nested namespaces and contain no retired package dependencies. ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [ ] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] 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>	2026-09-02 12:06:52 +00:00
Marc SchierandCopilot	cef83550cb	OPC UA Vision: a vision-guided bin-picking demo an LLM agent can see and drive (#4235) ... # Description Implements **OPC UA — Vision** and uses it to build the demo the companion specifications were drafted for: an eye-in-hand camera watches a bin of parts, a language model *looks at the frame*, decides which part matches the instruction, and commands the robot through Robot Intent — with the OpenUSD viewer showing the arm move and the bin empty out. > **Draft, and stacked.** [#4195](https://github.com/OPCFoundation/UA-.NETStandard/pull/4195) has merged and master is merged in, so what is left on top of `master` is the Vision work plus the unmerged `marcschier/ai-model-management` branch, which the demo needs for the model/dataset/deployment provenance. Review the Vision commits; the AI Model Management work lands with its own PR. ## What is here **The specification, implemented.** `Opc.Ua.Vision` is generated from the spec's own `Opc.Ua.Vision.NodeSet2.xml`, vendored byte-identically (SHA-256 `79C592C6…`, verified against a fresh download and stored LF-preserved like the Robot Intent NodeSet). `Opc.Ua.Vision.Server` builds the §4.2 discovery object, sensors, the coordinate-frame tree, calibrations, media endpoints, inference pipelines and the §9 feedback methods, with rendering and inference behind injectable providers. `Opc.Ua.Vision.Client` is built on the generated ObjectType proxies and composes transforms across the frame tree, which is what turns a detection in camera coordinates into a pose a robot can act on. **Two perception paths behind one contract**, which is exactly what `InferenceLocationEnum` is for. `OnServer` is a deterministic detector derived from the stage's ground truth — no model, no network, no GPU, so CI can exercise the whole loop. `EdgeOffServer` is the agent: it sees the frame over MCP and calls `SubmitDetections`, and the Server publishes results it did not compute. A client reads `DetectionResultType` identically either way. **The agent's eyes.** `tools/Opc.Ua.Mcp.Vision` adds 22 `vision_*` tools, which with the four shared connection tools makes the `vision` profile 26; `vision_get_frame` returns the camera image as an MCP `ImageContentBlock`, so the model looks at pixels rather than reading a description of them. Tool profiles now compose — `--profile vision,robotics` yields 62 tools — so one agent can both see and act without dragging in the full 136-tool catalogue. **The cell.** `samples/Robotics/BinPickingCell` follows the spec's own Robotics-Vision Addendum: the `world → robot_base → flange → gripper_tcp` frame tree with `camera_eih` on the flange, the `HandEye` extrinsics and the `Intrinsics612x512` intrinsics — the addendum's 2448×2048 calibration scaled to the 4×4-binned grid the camera actually delivers. `samples/Robotics/BinPickingClient` hosts the composed MCP catalogue and optionally the viewer. **Documentation.** `docs/Vision.md` (~800 lines) covers the model, hosting, the build context, every topology builder, both perception paths, feedback, the MCP profile and the limitations; linked from `docs/README.md`, with the profile table in `docs/McpServer.md` and READMEs for both samples. ## Evidence, not assertions Each of these was measured on a running system rather than assumed: | Claim | Evidence | |---|---| | The camera sees the bin | 612×512 frame with the bin and all five parts in view — red `(212,47,47)`, green `(61,193,71)`, blue `(54,85,218)`, yellow `(215,196,58)`, orange `(209,119,55)` | | An agent can act on what it sees | Over MCP, `vision_get_frame` returns a 612×512 PNG that decodes from base64 to 1,254,051 bytes, the descriptor beside the detections says 612×512, and all five `BoundingBox2D` centres land **inside** the frame on the pixel colour their own detection claims | | The geometry is right | A detection composed `camera_eih → world` lands on the authored position, residual **0.0000 m** — on both the on-server and off-server paths | | The detector tracks the world | Placing a part drops the next scan from 5 detections to 4, with that part absent | | The robot obeys the agent | Scripted run took command authority and completed a pick and a place, both `Succeeded` | | Tool catalogues are honest | Counted from a running server: vision 26, robotics 40, `vision,robotics` 62, full 136, **zero duplicates** | | Every node is browsable by NodeId | Eight representative NodeIds — both folders, a pipeline, a frame, the sensor, its Calibrations folder, the HandEye calibration, the Media folder — all `Good` | That tool-count check matters because the profile tests collapse names into a `HashSet` and *cannot see* a duplicate registration — so the new tests count registrations, not names. ## Things that were wrong, and are not now Writing the tests found three defects of one kind — code that answered instead of refusing. A zero-norm quaternion was silently rewritten to identity, so a caller with an all-zero orientation got a confident wrong grasp pose. `SubmitCorrection` rewrote a null `ResultId` to empty and called the sink anyway. A bad `StageIdentifier` **killed the host process** inside native `UsdStage.Open`, via an exception .NET cannot catch. All three now refuse loudly and are pinned by tests. **Nodes that existed but could not be reached.** The node manager indexes the Vision root before configurators run, so everything the fluent builder grafted on afterwards — both folders, the pipelines with their Results and Feedback, the frames, the sensors with their optics, calibrations and media endpoints — was never added to the index. Browsing *forward from a parent* worked, because that walks `NodeState.Children` in memory; browsing any of those nodes *by its own NodeId* returned `BadNodeIdUnknown`, which is how an ordinary client and the MCP discovery tools navigate. Registration is now deferred and flushed after each configurator. The first fix covered only the DI path and left the documented non-DI fallback broken in exactly the same way, so `ConfigureVisionAsync` now exists and registers what it builds. That defect survived a green suite because every test browsed children. The new tests assert by NodeId, and were checked against the broken code — with the flush removed, they fail. **Every Method was uncallable.** The Server passed 426 unit tests while not one of its Methods could be invoked over a real session — `RunInference` returned `Bad_TooManyArguments`, and so did every other Method that takes arguments, which is all of them but `StartContinuous` and `Stop`. Three independent faults, each sufficient on its own: the Method nodes were never created (they are Optional children, and the dispatcher guards every attachment with a null check, so it silently attached nothing); nothing declared the arguments, so the stack concluded none were expected; and `MethodDeclarationId` pointed at a synthesised NodeId rather than the NodeSet declaration a client calls with, so the lookup missed the instance Method entirely. The builder now creates the Methods, the `Results` folder and the `Feedback` object when a provider or sink is configured, declares every signature, and names the right declaration. Unit tests could not have caught this: they invoke the handler delegates directly and never go through `MethodState.Call`. It took an end-to-end test over a real session. Relatedly, children created by the generated `CreateOrReplace` helpers carry no `ReferenceTypeId`, so nothing could reach them — fixed once in the build context rather than at the several dozen call sites. **And one where I was wrong.** Earlier in this branch I reversed my own agents' decision and made `SubmitDetections` accept an empty detection set, reasoning that "I looked and the bin is empty" is a correct observation and refusing it forces a correct agent to invent a detection. The reasoning is sound; it is also an argument *against* the specification rather than an implementation of it. §9.5 is normative and explicit — its StatusCode table lists `Bad_InvalidArgument` for "`Detections` empty; or `SubmitCorrection` supplies both or neither corrected array". Worse, **none of §9.5's argument rules were enforced anywhere** — the dispatcher forwarded every submission straight to the feedback sink, so conformance depended on whichever sink a host installed. They are Server obligations, so they now live in the dispatcher, ahead of the sink, proven by strict mocks that would throw if the sink were consulted. The usability problem is real and does not go away by conforming, so it is raised where it can be fixed: [opcua-drafts#70](https://github.com/marcschier/opcua-drafts/issues/70). ## Tracking the updated drafts The Vision, Robot Intent and AI Model Management NodeSets on `opcua-drafts` `main` have moved, partly because of feedback this branch raised. All three are re-vendored byte-identically — verified by comparing git blob hashes against the upstream files rather than by eye — and the implementations follow. **Two statements that were previously inexpressible now have a wire representation.** `SubmitDetections` takes `SceneIsEmpty` and `SubmitCorrection` takes `RetractAll`, so an agent can report "I examined this frame and there is nothing in it" — the terminating condition of the bin-picking task — and can retract a false positive by correcting a result down to nothing. Both are checked in both directions, because the flag is exactly what separates a deliberate empty observation from a lost payload: an empty array without the flag is refused, and the flag with an array attached is refused as two contradictory claims about one frame. Implemented in the dispatcher, the client, the MCP tools the agent drives, and the sample cell. Also absorbed: `LampType` and `LightingMode` became enums (§66), `ProfileName` became `DefaultProfileName` on the endpoint (distinct from the `ProfileName` a caller passes to `GetStreamEndpoint`, which keeps its name), AI `Invoke`/`InvokeAsync` gained `PayloadUri` under the same exactly-one rule, and AI `ListModels` gained a `ContinuationPoint` — because `MaxResults` alone puts everything past the bound permanently out of reach. **OpenUSD is on `0.9.0-alpha`**, verified on the running cell rather than by restore alone: D3D12 backend, 612×512 frame, the bin and all five parts in view and detected — the same result `0.8.0-alpha` produced. The one local workaround that survives is `VisionMethodArguments`, and its reason has changed rather than gone away: the builder materialises these Methods as Optional children through `CreateOrReplace`, which constructs the state object directly and never runs the generated factory that now carries the signatures. Checked rather than assumed — removing the declarations fails five of the eight `VisionMethodSurfaceTests`. ## Review feedback Three threads, all addressed in `0fe2803a3`: - **The AI Model Management work is now a library family, not a sample.** `src/Opc.Ua.AI` (model), `.Inference` (`IInferenceBackend` and backends), `.Server` (node manager) and `.Client` (discovery, reads, calls, artefact transfer), in the same shape as the Robotics and Vision families; the samples become `samples/AI/ModelManagementServer` and `ModelManagementClient`. The client library needed real work rather than relocation: its browsing surface was `private` inside the sample's scenario runner, so moving the file alone would have produced a library nobody could call. - **No cloud-vendor SDK is referenced anywhere.** Inference goes through `Microsoft.Extensions.AI` — an `IChatClient` is implemented by hosted services and on-device runtimes alike, which is exactly the property clause 8.1 asserts. `Azure.Identity` is gone: workload identity is not an Azure feature, every platform projects a token into a file and each SDK reads that file behind its own API, so the resolver reads it directly. The comment claiming `Microsoft.Extensions.AI` would force a `System.Text.Json` bump was stale — it pins 10.0.10, which this repository already centralises on. - **`BinPickingCell` and `BinPickingClient` moved into `samples/Robotics/`**, which is where they belonged: the cell was already linking the arm, its kinematics and both USD assets out of `IntentEnabledRobot`. ## Upstream Work that belonged elsewhere went elsewhere rather than being worked around locally: - **`openusd-dotnet` [#13](https://github.com/marcschier/openusd-dotnet/pull/13)** — merged and shipped in `0.8.0-alpha`. `SilkFrameCapture.Capture` rendered only the first capture of a session and silently returned a blank frame for every one after it. Also filed [#14](https://github.com/marcschier/openusd-dotnet/issues/14) (no camera-by-prim API) and [#15](https://github.com/marcschier/openusd-dotnet/issues/15) (skipped tests reported as passed — which fooled me for several runs). Both are fixed; this branch takes `0.8.0-alpha` and deletes two workarounds, including 143 lines of hand-rolled projection maths. - **`opcua-drafts` [#66](https://github.com/marcschier/opcua-drafts/issues/66)–[#69](https://github.com/marcschier/opcua-drafts/issues/69)** — a close read of the spec produced 22 candidate ambiguities; 12 were discarded as misreadings with clause citations and the remaining 10 filed. One is concrete: five members exist in the NodeId CSV that no clause documents. - **`opcua-drafts` [#70](https://github.com/marcschier/opcua-drafts/issues/70)** — §9.5 could not express an empty observation or a false-positive retraction. **Adopted**, with the explicit-flag resolution argued for: `SubmitDetections` gained `SceneIsEmpty`, `SubmitCorrection` gained `RetractAll`, and the corrected-array rule relaxed from *exactly one* to *at most one*. - **`opcua-drafts` [#71](https://github.com/marcschier/opcua-drafts/issues/71)** — the Vision NodeSet browse-named all 13 `InputArguments` / `OutputArguments` Properties in the Vision namespace rather than the base namespace, so a stack could not find them and every Method in the specification was uncallable. **Fixed**; this branch now tracks the corrected artefact. ## The defect underneath it all The end-to-end suite was removed earlier in this branch because nine of its fourteen tests failed on one symptom: a client could not open a pipeline's `Feedback` object or enumerate its `Results`, even though browsing the same parent listed both. Restoring it meant explaining that, and the explanation was not in Vision. `RelativePathElement` initialised **`IsInverse` to `true`**. A caller that sets a `ReferenceTypeId` and a `TargetName` — which reads as *"the child reached by this Reference"* — was therefore asking the Server for the **inverse** Reference, and got `Bad_NoMatch` with nothing in the request to suggest why. `ObjectTypeClient.ResolveChildNodeIdAsync` is such a caller, and it is what **every source-generated Optional-child accessor in the stack** is built on — the accessor whose own documentation cites `AlarmConditionTypeClient.GetShelvingStateAsync` as an example. Vision is merely where it surfaced, because Vision puts two Optional children on the path an off-Server agent must take. The sibling helper in `StateMachineTypeClientExtensions` already set `IsInverse = false` by hand, which is the shape of a trap rather than a default. The default is now forward, `ResolveChildNodeIdAsync` says so explicitly, and the existing test that had recorded `true` as *expected* now pins `false` with the reasoning. Of the forty `RelativePathElement` construction sites in the tree, that accessor was **the only one relying on the default at all** — every deliberate caller already set it. Two further defects were found underneath it, both real and both fixed: - The generated `CreateOrReplace` helpers construct a child state object directly and leave **`TypeDefinitionId`** unset as well as `ReferenceTypeId`. A child referenced by nothing cannot be browsed; one with no type definition is a malformed Object that a client filtering by type silently skips — which is exactly what `EnumerateResultsAsync` does. Normalisation moved to `VisionNodeManager`, so it also covers the case the builder cannot see: a result published **at runtime** by an inference provider, long after the address space was built. That path produced results a client could list but not read. - `Opc.Ua.AI` shipped without an `AssemblyInfo`, so the solution built with six `CA1014` warnings. I had initially "fixed" this in the Vision client with Browse-based fallbacks, on the strength of a probe that appeared to show `TranslateBrowsePathsToNodeIds` failing even for `Server → ServerStatus`. That probe was wrong: it used an unqualified `ReferenceTypeIds`, which inside a `Opc.Ua.Vision.*` namespace binds to *Vision's* `ReferenceTypeIds` and yields a nonsense NodeId. Chasing that to the end is what turned up the real cause. **The fallbacks are gone** — they would have hidden a stack-wide bug behind a Vision-shaped workaround, and one of them matched on browse name while ignoring the namespace index. `tests/Opc.Ua.Vision.Intent.Tests` is restored and back in `UA.slnx`, passing **14/14** on its own merit rather than through a client-side workaround. A registration test now pins the pipeline's `Feedback` and `Results` children, which nothing covered before. ## Running it, and what that found The demo's premise is that a language model **looks at the bin and reasons about what it sees**. Running it end to end showed it could not, for reasons no test covered, because every one of them lived between components. **The sample did not compile.** Two `CS8604` nullable errors with `TreatWarningsAsErrors`, so neither the cell server nor the demo could be started at all. Worth recording that my first explanation was wrong: CI *does* build this project - the Linux leg's `net10.0` pass compiles it, log-confirmed - so this is not a coverage gap in the workflow, and no workflow change was made. **`Pick` and `Place` never moved the arm.** They were a delay plus a gripper action, so the operations reported `Succeeded` while nothing travelled. They now solve IK once and interpolate in joint space. **And the parts never moved**: the arm swung but the bin stayed full, because nothing tied the held object to the world state. The connector now creates 11 monitored items where it created 6, and a scripted run delivers `SeqNo=151` across 10 messages where it delivered `SeqNo=3` across 1. **The frame was smuggled through a string field.** The media provider put a base64 *data URI* of the PNG into `VisionImageReferenceDataType.Uri` - a field meant to be a *reference*, while the bytes were already returned in the inline `ByteString`. Every response therefore shipped the image twice, and a 1,311,400-byte PNG became a 1,748,536-character string against a `MaxStringLength` of 65,535: **26.7x over**. That is why raising `MaxByteStringLength`, `MaxArrayLength` and `MaxMessageSize` never helped - none of them govern a `String`. Separately the MCP tool assigned raw bytes where `ImageContentBlock` requires base64, so the wire carried `"data":"�PNG..."` - every non-UTF-8 byte replaced by U+FFFD, destroying the image and inflating the response to 6.8 MB. **The camera was aimed at nothing.** The arm's home pose, the flange scan pose the Vision model declared and the joint angles in the USD stage were three independent claims about where the camera was, and they disagreed - the declared flange orientation pointed the view 86 degrees away from the projection camera. Solving them as one thing exposed two further constraints that are easy to miss: the camera has to sit off the tool axis or it photographs its own gripper, and the IK branch has to be elbow-back or a link parks under the camera and fills the frame with the arm's own upper arm. A third was a regression this branch introduced and `Opc.Ua.Robotics.Tests` caught - aiming a straight-down camera from a point on the base's own X-Z plane puts the wrist on the J4/J6 singularity, so every motion away from home fails to solve. A 15-degree camera roll, free because the camera still looks straight down, gets 25 degrees clear, which is the margin the poses before this branch had. **The frame and the detections were different images.** The sensor declared 2448x2048, the clip endpoint 1280x1024, and the renderer produced 640x512 - not even the same aspect ratio. The ground-truth detector projects through the declared intrinsics, so its boxes lived in 2448x2048 space while an agent was handed a 640x512 picture: *"pick the red cube you can see"* arrived with coordinates that pointed off the image. Everything now agrees at 612x512, an exact 4x4 bin of the native sensor with the intrinsics scaled to match; the native size stays in the model and serial number, where it describes the hardware rather than the image. **`LatestClip` was created and never written**, so it reported `Bad_NoDataAvailable` for the life of the Server and a consumer following the model - read the published frame, call the method only if there is none - never got a frame. The dispatcher now publishes a clip it has just encoded, for every Vision server rather than this sample alone. One genuine API gap turned up on the way: `OpcUaServerOptions` exposes `MaxByteStringLength`, `MaxArrayLength` and `MaxMessageSize` and **no `MaxStringLength`**, so a Server that legitimately needs a long string could not raise the limit through the hosting API at all. Added, with tests. It is not the fix for anything above - that was a design bug - but the missing knob was real. ## Not finished - **Two projects sit below the repository's 80% line coverage.** `Opc.Ua.Vision.OpenUsd` is at 71.1% — what is left only executes with a real OpenUSD stage and plugin tree, which is not on every build agent. `Opc.Ua.AI.Inference` is now at **86.7%**, up from 12.9%: the code this branch *wrote* is covered by 25 tests, and `RestChatCompletionsBackend` — previously the whole of the shortfall — is covered by 18 more against a stubbed `HttpMessageHandler`, exercising success, refusal, throttling, retry-after, timeout and cancellation without a network. Everything else clears the bar — `Opc.Ua.Vision.Server` 92.1%, `Opc.Ua.AI` 91.4%, `.Vision.Client` 88.1%, `Opc.Ua.Vision` 85.7%, `Opc.Ua.AI.Server` 80.1%, all measured with coverlet. The CI coverage gate reports itself as advisory and non-blocking. - **`SamplesCollected` is not counted here, and `docs/Vision.md` now says so.** Section 9.4 requires a Server to count a negative example (`SceneIsEmpty` / `RetractAll` carrying a `GroundTruthLabel`) exactly as it counts one carrying geometry. `VisionNodeManager` does not, and structurally cannot: `SamplesCollected` is a property of `LearningJobType`, which the *AI Model Management* companion defines, and Vision reaches the job through a `NodeId` value rather than a Reference **precisely so this model takes no dependency on the model that defines it**. What the Server does guarantee is that the negative example survives the hop intact — both flags are carried verbatim to `IVisionFeedbackSink` — so a host that binds a learning job has everything it needs to satisfy the clause on the counter it owns. Stated in the limitations rather than left implied. ## Related Issues Was stacked on #4195, now merged. No tracking issue yet for the Vision implementation itself — happy to open one if you would like the design recorded as an ADR before this goes further. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. — *Types 8562, Server 4072, Client 2123, InformationModel 1063, Robotics 540, Tools 466, Vision 442, AI 74 and the restored Vision+Intent end-to-end suite 14/14, all green on net10.0. Only `.OpenUsd` (71.1%) is below the 80% bar — see Not finished.* - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. — *`dotnet build UA.slnx -c Release`: 0 errors, 0 warnings.* - [x] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. — *net48: Types 8555, Client 2126, Vision 385 green. Two net48 Server tests fail (`CreateUserManagementSeamBindsTheModelToTheServerAsync`, and `DurableDataValueQueueVerifyReferenceBatchingAsync` only under parallel load); both were confirmed **pre-existing** by re-running them with the `IsInverse` change reverted, which fails identically.* - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. — *The net48 build legs failed because the restored end-to-end project used `ValueTask.FromResult` / `string.Create`, which do not exist on .NET Framework; fixed in `fb57522da` and verified building on every TFM. `DisposeDrainsHeldConnectionCallbackBeforeListenerDisposal` failed once on the Ubuntu Client leg — a 13 ms timing-sensitive reverse-connect disposal test that passes locally 2123/2123 and touches no browse paths.* - [ ] I have addressed **all** PR feedback received. ## Typed Vision-guided Robotics MCP workflows The branch now also makes the demo workflow first-class in the Full MCP profile: - replaces stringified Robotics intent and mission inputs with typed request schemas and exact name-or-NodeId selectors; - adds bounded operation/mission paging, stable step-to-operation identity, retained mission discovery, and `robotics_wait_mission`; - makes `vision_run_inference` return bounded typed detection, inspection, or segmentation summaries in one call; - adds same-session `robotics_vision_pick` to select a detection and submit Pick or Pick/Place work without implicitly taking authority, retrying, cancelling, or waiting; and - starts Publish workers for classic subscriptions so an independently launched OpenUSD viewer receives live robot and part updates. The migration guide documents the intentionally breaking MCP request-schema changes. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dab1302-19c5-4a9b-a50c-d97d389713aa Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b Copilot-Session: 99d1ab42-f83a-41ab-bf3e-85bf81fadace Copilot-Session: bcbc8ac9-5abd-42a8-8bac-e10533790e2e	2026-08-29 19:34:15 +02:00
Marc Schier	06ae192084	Fix CI failures tracked by #4299 (#4308) ... ## Summary - stabilize the History, authentication lockout, Robotics, redundancy, and DI tests reported in #4301-#4305 - fix the closed-transport buffered-write completion deadlock that hung `ConnectMultipleSessionsAsync` - add macOS crash diagnostics for #4306 and retain evidence for root-cause analysis - run macOS tests and NativeAOT in GitHub Actions pull requests, using supported Intel and ARM64 runners, while keeping every Azure macOS definition disabled by default - move all remaining source-level `InternalsVisibleTo` declarations into project files so signed OpenUSD builds receive the repository public key ## Local validation - MCP PubSub: 29 passed - History focused tests: 3 passed - Sessions lockout: 3 passed - Sessions multiple-session regression: 3 passed - deterministic binary client channel tests: 22 passed - Robotics runtime: 34 passed - Redundancy focused tests: 4 passed - OpenUSD net10.0: 914 passed - affected OpenUSD projects build on net10.0 and net48 Refs #4299, #4300, #4306 Fixes #4301 Fixes #4302 Fixes #4303 Fixes #4304 Fixes #4305 --------- Copilot-Session: 7cb486de-8b4c-43c5-a900-e7d2e0ad04a8	2026-08-22 19:30:19 +00:00
4 people	d525bc3f28	Bound reconnect cleanup and organize samples by domain (#4276) ... # Description This PR fixes bounded ManagedSession reconnect cleanup and reorganizes the sample applications into domain-focused folders. ## Reconnect cleanup - Bounds reconnect integration cleanup with independent 30-second close and dispose limits. - Sends `CloseSession` through the awaited transport request path during `Session.DisposeAsync`. - Preserves explicit `Session.CloseAsync` behavior that waits for or cancels outstanding publish requests. - Covers the disposal request path with a regression test. ## Sample organization - `samples/OpenUsd`: `GeneratorServer`, `SiteCompositionServer` - `samples/Redundancy`: `RedundantClient`, `RedundantServer`, `RedundantPubSub` - `samples/Isa95`: `MinimalIsa95Server` - `samples/MinimalApi`: `MinimalBoilerServer`, `MinimalCalcServer`, `MinimalClient` - `samples/DI`: `PumpDeviceIntegrationServer` - `samples/Lds`: `ConsoleLdsServer` - `samples/Reference`: `ConsoleReferenceClient`, `ConsoleReferenceServer` - `samples/PubSub`: `ConsoleReferencePubSubClient` `UA.slnx` now exposes the same requested domain groups. Project references, tests, runtime sample probes, CI workflows, Dockerfiles, Compose files, helper scripts, and documentation were updated to the new paths. `samples/Robotics/MinimalRobotServer` remains under `Robotics`. ## Validation - `dotnet build UA.slnx` - 3 Redundancy short-haul sample tests - 70 DI sample tests - 914 OpenUSD tests - 1 WotCon model-materialization test - 137 AOT tests - `docker compose config` for all 6 moved Redundancy Compose files ## Related Issues - Fixes #4273 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce new build or analyzer warnings. - [x] I built the complete `UA.slnx` solution locally. - [ ] I ran all tests locally against at least .NET Framework and .NET 10. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [x] I have addressed all PR feedback received. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: marcschier <11168470+marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <mschier@microsoft.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Copilot-Session: a2c2838f-74ae-4997-a1d4-50574d9e0636	2026-08-21 04:35:47 +00:00
3 people	466c5cb982	Robot intent follow-ups: multi-controller executors, spec fixes, and an MCP tool library (#4195) ... Follow-ups to #4165, plus the work that followed from re-checking this branch against the specification. Everything started as a defect found by review or by probing the running code; the specification is unchanged except where noted. The TLS certificate lifetime fix (#4196) and the OpenUSD work (#4198) were both merged into this branch. #4196 has since landed on master, so it no longer appears in this diff. #4198 was closed in favour of this PR and its content is here. The MCP trim/AOT work (#4228), the `DataGenerator` null `NodeId` fix (#4227) and the NodeSet external-parent import fix (#4229) have since been merged in as well. All three are closed in favour of this PR and their content is here, described under *Making the MCP libraries trim and AOT clean*, *A generator that returned the value you opted out of* and *A NodeSet parent that lived outside the import batch* below. ## A two-arm cell could not be built `IntentExecution` carried no controller identity, and there was effectively one `IIntentExecutor` per server. An executor handed an execution therefore could not tell **which arm** it was for, so a cell with two `IntentControllerType` instances — an ordinary configuration — was not expressible through the builder or DI. Both halves are addressed: - `IntentExecution` now carries `ControllerId` (`NodeId`, the OPC UA identity) and `ControllerName`, so one shared executor can discriminate; - DI gained per-controller registration — `AddRobotIntentExecutor<TExecutor>(controllerBrowseName)` and an instance overload — so two genuinely different executors can coexist. Resolution order is `WithExecutor` → per-controller DI registration → the single DI executor → a build-time throw. There is still **no silent fallback** to a rejecting executor; that was a Critical defect fixed in #4165 and it stays fixed. `IIntentExecutor` is unchanged, so this is purely additive. ## Declarations that were quietly untrue - **`Accepts<T>` silently discarded a redeclaration.** Declaring an intent type twice — say to narrow `cancelSupported` on the second call — kept the first, wider declaration and said nothing. That is the same class of defect as the capability-honesty problems #4165 fixed (`PauseSupported` declaring a pause the host could not perform; `Aborting` degrading silently to `Buffered`). Identical redeclarations remain idempotent; conflicting ones now throw `BadInvalidArgument`. - **`RI-Interop-40010` assumed Annex B item 3 rather than checking it.** It now requires exact set equality between the OPC 40010 `TaskProgramName` values and the Robot Intent `ProgramType.ProgramId` values. Both sides are readable from the address space without commanding the robot, which is what makes the requirement structural under §12.1. ## Two things the specification requires that this did not do Re-checking the implementation against the merged draft revision turned up two genuine violations. Both are fixed, and the NodeSet is untouched by either. - **Every graphed mission reported `Failed` on success.** `MissionRules.SelectTransition` returned `null` for two different situations — a graph *terminus*, which is the normal success path, and a branch point whose conditions all evaluated false — and the caller collapsed both into `Failed`/`NoTransition`. A three-way result (`Terminus` / `Selected` / `NoConditionMatched`) separates them, so a mission that runs to its end now completes. The failure reason surfaces through the inherited Part 10 result container rather than a new member. - **Facet URIs were never published.** An upstream `may` became a `shall` in the revision this branch adopts. `RobotIntentConformanceUris.Facets` held 31 URIs with **zero** production references — the set existed but nothing emitted it. The published set is now derived from `RobotIntentFacetCalculator`, the same authority that already computed `SupportedFacets`, rather than from a second hand-maintained list that could drift; a reflection test pins the two together. ## An easy client mistake `RequestAuthorityAsync` returns a lease the caller must remember to inspect. Forgetting makes every later submission fail with `ControlNotOwned` for reasons that are invisible at the call site. `RequireAuthorityAsync` throws instead, and names the current owner. `RequestAuthorityAsync` is untouched — inspecting and branching is legitimate. ## Letting an agent drive the robot New `tools/Opc.Ua.Mcp.Robotics` gives a language model the intent API as MCP tools: 36 across discovery, monitoring, direct control and missions, behind a `Robotics` tool profile, composable into any MCP host exactly as the existing OPC UA tool packages are. The `IntentViewerClient` sample hosts them behind `--mcp` (stdio by default, HTTP when `--view`, since MCP stdio and the in-process viewer would otherwise share stdout), and the sample gained a pallet scene so an agent can pick and stack parts and watch the result in the OpenUSD viewer. All the scene logic is server-side — no viewer or connector changes. Four gaps in the client API had to be filled first, since the tools are a thin layer over it and could not have been written honestly without them: `ReadStateAsync`, `ListOperationsAsync`/`ListMissionsAsync`, controller-level `CancelIntent`/`CancelAll`/`Pause`/`Resume`/`Retry`/`ReleaseControl`, and `WaitForCompletionAsync`. The tools refuse rather than paper over: a refusal from the server is returned with its `IntentFailureEnum` and never retried, and command authority is never acquired as a side effect of a tool call. **One defect this surfaced:** `--profile robotics` shipped 36 tools that could not do anything. Every robotics tool resolves a named OPC UA session and only the connection tools can open one, so with no `Connect` in the catalogue the whole profile failed. A test had pinned the defect in place, asserting the profile did *not* contain `Connect` on the line below the `diagnostics` assertion requiring it. Each package now declares its own requirement in its registration extension rather than the host special-casing it, so an application embedding `Opc.Ua.Mcp.Robotics` directly gets a usable catalogue without reproducing the host's pairing. `Full` still takes the connection tools from the core package, so they are never registered twice. The replacement test states the invariant — a profile exposing session-scoped tools must expose the tools that open a session — rather than the instance. ## Making the MCP libraries trim and AOT clean *Merged from #4228.* `Opc.Ua.Mcp.Core` is now `IsAotCompatible` on `net10.0`, and the reflection the analysers then reported is gone. Turning the analysers on is the point: until now they were not running on these libraries at all — including on the tool library this branch adds. - **`OpcUaJsonHelper.Serialize`** went through `JsonSerializer` (IL2026 / IL3050). The conversion helpers in that class already reduce every OPC UA value to a closed set — `null`, the primitive scalars, `string`, `Dictionary<string, object?>` and lists thereof — so the JSON is now written directly with a `Utf8JsonWriter`. Output is unchanged, and that is asserted rather than assumed: `OpcUaJsonHelperTests.SerializeMatchesTheReflectionBasedSerializer` compares the new writer byte for byte against `JsonSerializer` for every shape the helpers emit, including all numeric widths, the custom-formatted scalars, nested sequences and strings that force the encoder to escape. One deliberate contract change: arbitrary object graphs are no longer accepted. Serializing them needs reflection, so `Serialize` now throws `NotSupportedException` naming the type instead of silently stringifying it. No call site in the repository passes anything outside the closed set. - **`McpSchemaFilters`** used `JsonSerializer.SerializeToElement` on a `JsonObject`; it now parses the node's own text, which needs no reflection. `Opc.Ua.Mcp.Diagnostics` is deliberately **not** marked. It depends on SharpPcap and on the reflective service-call dissection in `Opc.Ua.Core.Diagnostics`, neither of which is trim safe — which is why that package does not opt in either. Two improvements are kept regardless: `PacketDecodeTools` serializes `DecodedServiceCall` through a source-generated `JsonSerializerContext`, and `PacketCaptureTools.ListInterfaces` now carries the `RequiresUnreferencedCode` / `RequiresDynamicCode` annotations that `NetworkInterfaceEnumerator.ListLocalInterfaces` already declares, so the constraint is visible to callers instead of implicit. Nothing is suppressed. Marking an assembly `IsAotCompatible` only proves its own IL is clean; it says nothing about whether the library still functions once reflection is gone. So `Opc.Ua.Aot.Tests` references `Opc.Ua.Mcp.Core` and exercises it with JSON reflection disabled, stating what is and is not true today: - tool **results** serialize without reflection — this would fail outright if the `Utf8JsonWriter` change regressed; - tool **registration** still requires reflection, because the MCP SDK builds each tool's input schema by asking `JsonSchemaExporter` for a `JsonTypeInfo` of every parameter type. Without a source-generated context covering the tool signatures — and with the injected `OpcUaSessionManager` being described rather than treated as a service — that throws `NotSupportedException`. The second test asserts the throw. It documents the remaining gap and turns red the moment someone closes it, rather than leaving it to be found in a published binary. The package README says the same thing in prose. **Two pre-existing issues this surfaced, both outside this PR:** pulling `Opc.Ua.Mcp.Diagnostics` into an AOT publish reports `IL2046` / `IL3051` on `NicCaptureSource.StartAsync`, which is annotated `RequiresUnreferencedCode` / `RequiresDynamicCode` while the `ICaptureSource.StartAsync` it implements is not, and `IL2075` on `ServiceCallReassembler.GetCollectionCount` / `GetPropertyString`, which call `Type.GetProperty` on the result of `GetType()`. Happy to file these separately if useful. ## A generator that returned the value you opted out of *Merged from #4227. Fixes #4226.* `GetRandomNodeId(useBoundaryValues: false)` could still return `NodeId.Null` — one of the boundary values the same method offers when boundary values *are* requested. A caller that opted out should not receive one, and `GetRandomExpandedNodeId` builds on it and inherited the same behaviour. The `IdType.Opaque` branch is the practical path: `GetRandomByteString()` draws its length with `NextInt32(MaxStringLength)`, which is inclusive of zero, and a zero-length opaque identifier in namespace 0 satisfies `NodeId.IsNull`. With three namespaces and `MaxStringLength` at 10 that is roughly **one call in 130**. A numeric identifier of `0` in namespace 0 is null too, at around one in four billion. The draw is now repeated while the result is null and boundary values were not requested. Guarding the result rather than the individual branches covers every identifier type, including the numeric case, rather than only the one that happens to be reachable today. The `useBoundaryValues: true` path is untouched, so boundary values remain available when they are asked for. Four tests in `tests/Opc.Ua.Core.Tests/Types/DataGeneratorTests.cs` loop 20,000 draws, because at one call in 130 a single draw proves nothing. Against the **unmodified** generator they fail on draw 93 and draw 1346. Two of the four exist to keep the fix honest rather than to test the bug: `GetRandomNodeIdStillProducesEveryIdentifierType` catches a "fix" that simply stopped emitting opaque identifiers, which would satisfy the null assertions while silently narrowing the generator, and `GetRandomNodeIdWithBoundaryValuesStillYieldsThem` confirms the leak was closed rather than the feature removed. **How it was found:** an intermittent downstream test failure. A test server initialising a static `NodeId`-valued variable via `TestDataSystem.ReadValue` occasionally stored `NodeId.Null`; the encoder correctly writes that as JSON `null`, and a consumer asserting a value was present failed about one run in five — never reproducibly in isolation. It only surfaced under load because `UnsecureRandom.Shared` is a fixed-seed, process-global generator shared between `DataGenerator` and OPC UA transport jitter, so the number of draws preceding address-space initialisation depends on what else is running in the process. ## A NodeSet parent that lived outside the import batch *Merged from #4229.* `UANodeSetHelpers.LinkParentChildRelationships` builds its lookup table from **only the nodes in the current import batch**, links a child when the parent is found — and then cleared `instance.Handle` **unconditionally**, which is exactly where `Import` had stashed the authored `ParentNodeId`. A node whose parent lives in another NodeSet, or is owned by another NodeManager, therefore lost that parent **silently** at import, and nothing downstream could recover it. `RuntimeNodeSetNodeManager.CreateAddressSpaceAsync` already has a step that wires references to external node managers; the parent link never reached it. Such a parent is now recorded rather than discarded, and exposed through `UANodeSet.TryGetUnresolvedParentNodeId`. A parent that resolved normally inside the batch is **not** recorded, so a caller does not re-wire references that already exist. The record lives in a `ConditionalWeakTable`, so it does not keep an imported node alive and does not widen `NodeState` — whose `Handle` is a general-purpose slot a caller may legitimately use for its own purposes, and so is not a safe place to leave the value. `RuntimeNodeSetNodeManager` consumes it to report the one case that cannot be repaired automatically. In NodeSet2 the `ParentNodeId` attribute is a *hint*; the authoritative edge is the entry under `<References>`, which the reverse-reference pass already turns into an external reference. A node that names an out-of-manager parent but carries **no such Reference** materializes with no path from that parent, and no ReferenceType can be inferred to synthesise one — so it is reported as an authoring defect rather than guessed at or silently accepted. This is deliberately **not** a fix for a specific address-space symptom. It removes a silent loss of authored information and gives callers the means to act on it. Whether any particular orphaned-node report is caused by this path, or by namespace-index translation elsewhere, still needs separate diagnosis. **Changed while merging.** As authored in #4229 the accessor was `GetUnresolvedParentNodeId`, returning `NodeId?`. `NodeId` is a readonly struct implementing `INullable`, so that was a `System.Nullable<NodeId>` wrapped around a type that already carries its own null representation — which this repository excludes in favour of an `IsNull` check, and it asks for `TryGet` over casting on struct types. The tests correspondingly asserted `Is.Null` against a struct, which the testing rules also exclude. It is now `bool TryGetUnresolvedParentNodeId(NodeState, out NodeId)`, yielding `NodeId.Null` when there is no unresolved parent. The API is unshipped — it arrives with this branch — so nothing depended on the previous shape. Both tests were re-verified by mutation after the rewrite: disabling the record in `LinkParentChildRelationships` still fails `ImportKeepsAParentThatIsNotInTheSameBatch`. ## Dependencies - **OpenUSD to `0.7.0-alpha`.** All 11 issues we filed upstream are fixed, so the workarounds they justified are gone and the parts of the pump and generator samples that were blocked on them are implemented. - **ModelContextProtocol to `2.1.0`** across all five MCP tool libraries and the executable. No code changes were needed — the surface we use is unchanged in 2.x — but central package management required a cascade of pins, each commented where it sits. Both transports were verified against a running server rather than assumed. MCP 2.x is stateless (no `initialize`, no `Mcp-Session-Id`); an OPC UA session is application state addressed by a caller-supplied name, which fits that model but does assume process affinity, and that assumption is now written down rather than implied. ## Infrastructure The `code coverage` gate was measuring nothing on this branch and reporting success. It is fixed and now enforced, which immediately exposed real gaps — most usefully `RoboticsIntentJson`, which parses agent-supplied JSON (untrusted input from a language model) and had no tests at all. It is now at full line coverage in CI. Patch coverage is **90.20%** against a 75% floor. The `netstandard2.1` build leg was also failing. The MCP projects build as empty shells under a legacy `CustomTestTarget`, and the viewer sample referenced them on the target framework alone, so it tried to resolve a net10.0 shell from net8.0. The sample now derives MCP support from whether those projects are actually being built. ## From review A review pass over the branch found two real defects, both fixed. `RoboticsIntentJson` parses JSON written by a language model, so every malformed shape it can be handed is reachable input; it indexed pose arrays without checking their length and enumerated properties without checking their kind, so a short or wrongly-typed array escaped as `IndexOutOfRangeException` or `InvalidOperationException` rather than the argument error the tool descriptions promise the agent. Separately, `IntentOperationHandle` treated a zero `QueuePosition` as "not observed" and discarded it, so a handle kept reporting the last queued position after the server had said the operation left the queue. The pose validation immediately caught a latent test defect: the pallet scenario interpolated doubles into JSON with the current culture, so under a comma-decimal culture `[0.12, 0.16, 0.28]` became a six-element array. It passed only because that submission is expected to be refused for want of authority, so the wrong coordinates were never used. A third review finding — that a default-constructed `RobotIntentControllerState` hands out a null `NodeId` — was checked and rejected: `NodeId`, `QualifiedName` and `LocalizedText` are readonly structs here, so `default` is the null value and `IsNull` is `true`. The tests asserting that invariant are kept. A second review pass over the merged branch found two more, both fixed and both about an error surfacing as the wrong kind of failure rather than about a wrong result: - **Malformed agent JSON did not arrive as an argument error.** `RoboticsIntentJson` turns every *validation* failure into an `ArgumentException` — a wrong root kind, a missing property, a short pose array — but three `JsonDocument.Parse` calls and the NodeId parse were unguarded. A syntax error escaped as `JsonException` and an unparseable NodeId as `ServiceResultException`, which are the two shapes a language model is most likely to produce. That is the same defect class as the pose/kind checking above: a caller separating bad input from a genuine fault had to know to catch three unrelated exception types. Parsing now goes through one helper that reports a syntax error as an `ArgumentException` naming the argument, keeping the `JsonException` as the inner exception so the position is not lost, and the NodeId goes through `NodeId.TryParse` rather than `Parse`, collapsing its two failure modes into one. `MalformedIntentJsonIsRejected` had asserted `InstanceOf<JsonException>`, pinning the behaviour rather than the intended contract; it now asserts `ArgumentException` like every neighbouring test, and three tests cover the newly guarded paths. - **`ConditionalWeakTable.Add` throws on a duplicate key.** Not reachable today, but `TryGetUnresolvedParentNodeId` documents `Handle` as a general-purpose slot a caller may use for its own purposes — and a caller doing exactly that before re-importing into the same collection would take a hard throw from inside the import pipeline. Removing first makes the most recent import win. `AddOrUpdate` would say this more directly but does not exist on `netstandard2.0`. ## Deliberately not fixed here `FastenIntentDataType.Joint` is refused when the Server exposes no OPC 40450/40451 model, and a client cannot discover that before calling. That is **not fixable in this repository**: §5.4.2 makes `Joint` the defining mechanism while §11.3 says the parameters otherwise stand alone, and `IntentCapabilityDataType` declares capabilities at intent-type granularity with no member-level mechanism anywhere in the model. It is a specification gap, raised as S4 on [marcschier/opcua-drafts#60](https://github.com/marcschier/opcua-drafts/pull/60) rather than worked around with a bespoke mechanism the spec does not define. ## Validation `dotnet build UA.slnx -c Release` on the merged branch — **0 errors, 0 warnings**, with the AOT analysers now running over `Opc.Ua.Mcp.Core`. | Suite (net10.0) | Tests | |---|---| | `Opc.Ua.Core.Tests` | 4247 (86 skipped) | | `Opc.Ua.Robotics.Tests` | 540 | | `Opc.Ua.OpenUsd.Tests` | 909 | | `Opc.Ua.Tools.Tests` | 419 | | `Opc.Ua.Server.Tests` — `RuntimeNodeSet` | 94 | | `Opc.Ua.Robotics.Intent.Tests` | 22 | | `Opc.Ua.Aot.Tests` — `McpAotTests` | 3 | 0 failures. `Opc.Ua.Tools.Tests` rose from 409 to 419 with the serializer-equivalence tests from #4228 and the malformed-JSON tests from the second review pass, and `Opc.Ua.Core.Tests` covers the `DataGenerator` tests from #4227 and the import tests from #4229. `UANodeSetHelpers` also passes on **net48** (5/5). The MCP agent-usability grader scores Core 100 / Full 100. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b Copilot-Session: 9e6a5abf-3299-4cd1-9855-010fedbf0ad8 Copilot-Session: a0718e8a-a158-45c5-929d-e1d6c0982431	2026-08-11 13:03:36 +02:00
Marc SchierandCopilot	2b44fcad22	Robot intent: a task-level command API with a Part 10 lifecycle (#4165) ... # Description Implement the draft **OPC UA — Robot Intent** companion model end to end: task-level motion verbs, a Part 10 lifecycle, a server hosting surface, a client, two samples and a live integration suite. It replaces the opt-in convention API merged in #4127 with an implementation generated from a companion information model. ## Why OPC 40010-1 *Robotics* describes robot **topology** in detail and defines **no motion verbs at all** — its entire actuation surface is `Start`, `Stop` and loading a named program. A conformant client can discover everything about a robot's construction and cannot ask it to move anywhere. #4127 filled that gap with ten verbs in an application-owned namespace, resolved by BrowseName, and the file said what it was: *"opt-in, explicitly non-normative industrial operation conventions"*. That contribution established a **vocabulary**. It established no **lifecycle**, and the lifecycle is the harder half. A motion takes seconds; a pick takes a minute. OPC 10000-4 §5.12.2 discards a method result when the Session ends *"independent of the task actually performed at the Server"* — so a synchronous motion method does not merely time out, it **loses the outcome of work that has already physically happened**. OPC 10000-10 §4.1 gives the OPC Foundation's own resolution: a Method performs a calculation, a **Program** runs a batch process or a machine tool part program. So `SubmitIntent` returns as soon as the intent is *admitted*, and what it returns is a NodeId — an `IntentOperationState` (a Part 10 program instance) the client subscribes to for progress and reads for the result. ## What changed **Model and shared code** — `Opc.Ua.RobotIntent.NodeSet2.xml` is source-generated in `Opc.Ua.Robotics`, so the enums, the polymorphic intent structures, `IntentOperationState : ProgramStateMachineState`, the method states and the typed clients derive from the model rather than being hand-written. Its only `RequiredModel` is the base UA namespace, so a server can adopt it without OPC 40010 or DI. `PoseMath` implements the specification's Annex C conversion between its unit quaternion and the core `ThreeDFrame` — including the `asin` clamp that stops a pole orientation becoming a domain error, and the non-negative-`w` representative that makes two servers agree on four numbers. `FrameTree` composes transforms along the frame tree. **Verbs are a DataType hierarchy, not one Method each.** A single submission and a mission step are then the same shape, and a new intent is a subtype rather than a new method — which is what `AddOperation<TRequest,TResponse>` was working around. **Server** — `IntentControllerHost` owns admission in the specification's order, the queue with PLCopen buffer modes, blending, cancellation *with the server's right to refuse*, missions with a committed base and a revisable horizon plus an IEC 61131-3 step graph, safety-aware refusals, real-time channel brokerage, and the capability declaration. All thirteen Methods are wired; command authority and channel leases are released when the holding Session closes; every client-supplied NodeId is validated against the controller being commanded (clause 11.3). `RobotIntentNodeManager`, the fluent `IIntentControllerBuilder` and `AddRobotIntent`/`ConfigureRobotIntent` make it reachable, and `RobotIntentFacetCalculator` computes which of the specification's facets an instance actually satisfies. **Client** — `RobotIntentClient` discovers `Server/RobotIntent/Controllers` and reads what a robot accepts; `IntentOperationHandle` subscribes and completes on a terminal state, recovering the result after a reconnect because clause 6.7 promises it survives; `CommandAuthorityLease` and `RealTimeChannelLease` manage the two exclusive resources; fluent builders cover every intent and mission. **Samples** — all robotics samples move under `samples/Robotics/`. `IntentEnabledRobot` is one UR5e-style offset-wrist arm on a bench, declared completely enough to show what a conformant server publishes, with a simulated safety source so the clause 10.4 refusals are demonstrable. `IntentViewerClient` turns a **click on a target prim in the OpenUSD viewport** into an intent and watches the arm execute it; it also runs headless, which is how CI exercises it. **Intents execute serially**, which satisfies every `BlockingMode` constraint by construction: the specification forbids beginning a `Single` or `Hard` intent while another executes and merely *permits* `None` and `Soft` to overlap. The convention API (`RoboticsOperationConventions`, `RoboticsOperationsBuilders`, `RoboticsOperationsClient`) is removed. ### On safety — what this deliberately does *not* claim The interface is **non-safety-rated**, and that is a property of the technology rather than a scoping choice. OPC 10000-15 carries cyclic safety data from a SafetyProvider to a SafetyConsumer; the consumer's `RequestSPDU` holds an identifier, a monitoring number and one octet of explicitly **non-safety** flags, so a caller has no channel through which to supply safety-rated arguments. Every safety fieldbus (PROFIsafe, CIP Safety, FSoE, openSAFETY) expresses a safety command as a **continuously asserted cyclic signal**, because the integrity argument rests on the fail-safe state that follows when assertion *stops* — and a Method call has no defined behaviour when it stops being called. What the host does instead is **observe and refuse**: `UpdateSafetyState` is how the application reports what the safety system is enforcing, and admission then refuses on the same values a client can read, so a refusal is explainable from the address space rather than from Server-internal state. It may never *instruct* the safety system, and no Method here commands a safe motion function, changes an operational mode or clears a stop. ## Defects this found in the specification, fixed upstream Implementing a specification is the only way to find out whether it can be implemented. Seven defects went back to [marcschier/opcua-drafts#47](https://github.com/marcschier/opcua-drafts/pull/47) rather than being worked around here: - **Refusal was unobservable.** Clause 6.2 orders six distinct refusals and clause 5.8 calls the failure set "small and diagnosable on purpose" — but `SubmitIntent` returned only `IntentId` and `Operation`, so a client could see *that* something was refused and never *which*. It now returns `Accepted`, `Failure` and `Message`, and clause 6.2 states that a refusal returns `Good` rather than substituting a Bad `StatusCode`. - **The Part 10 promotion was illegal.** `IntentOperationType.ProgramDiagnostic` was declared as a Property of `PropertyType`; OPC 10000-10 declares it a Variable of `ProgramDiagnostic2Type` reached by `HasComponent`. The declaration added a *second* member beside the inherited one instead of promoting it — which is precisely why an earlier version of this branch had deleted it. Correcting the TypeDefinition made it generate cleanly with **no change to the curated Core**. - **Clause 9 said nothing about the Method surface.** A server could declare `MissionsSupported` true and omit `SubmitMission` entirely, because the mission and channel Methods are Optional. This implementation did exactly that until an integration test caught it. A fourth honesty rule and a table now fix which Methods each declaration implies. - **Conformance was not machine-readable, and two implementations disagreed.** Clause 12 defined conformance in terms of facets and gave a Server nowhere to publish which ones it satisfied, so every client had to re-derive Table 12.2 from the address space. This repository proved why that fails: the server''s `RobotIntentFacetCalculator` and the client''s `RobotIntentRules.DeriveFacets` each implemented clause 12.2 independently and **disagreed on essentially every row** — the client''s `Blending` was `BlendingSupported` alone where the server also requires all four blending buffer modes, its `QueuedIntents` was `MaxQueueDepth > 0` where the server also requires `Buffered`. A client and server from the same release answered "does this controller conform?" differently, and both were reading the specification correctly, because several rows are behavioural and no amount of browsing settles them. `IntentCapabilitiesType` now carries **`SupportedFacets`**, exactly as OPC 10000-5 does with `ServerCapabilitiesType.ServerProfileArray`, RI-Base requires it, and clause 12.2 separates structural requirements a Server shall meet before listing a facet from attested ones governed by clause 9. - `WaitIntentDataType.Signal` was unbounded where clause 11.3 has to check it; §11.3 gained a table fixing the expected type of every NodeId-valued member; §5.7.0 gave `Ready`/`ActiveIntent`/`ActiveMission` normative meaning; §6.9 bounded `RequestedLease`; §6.5 and §6.4 now say what a server that cannot differentiate `StopMode` must do, and which stop a superseded intent gets. ## Bugs the tests and reviews caught Each of these would have been silent in the field: - **The execution engine was never wired into the public API.** `IntentControllerBuilder.RegisterAsync` built the address space and never started an `IntentControllerHost`, so through the documented `AddRobotIntent` → `ConfigureRobotIntent` path **every Method returned `BadNotImplemented`** and the registered `IIntentExecutor` was never resolved. A browsable address space is not a working robot interface, and only a live integration test showed the difference. - **NodeId scoping failed open on an empty index.** A controller advertising `CallProgramIntentDataType` with no published `ProgramType` accepted an *arbitrary* NodeId as `CallProgram.Program` — and `CallProgram` runs code the server holds. - **Blending completed the predecessor at submission time** and abandoned its motion, telling the client it succeeded and publishing an all-zero pose at the origin as a genuine `AchievedPose`. - **Mission base immutability compared only identifiers**, so a client could keep the ids and swap the *motion* of an already-committed step. - **`FinalResultData/Result` was never registered** with the node manager: Browse showed it, Read returned `BadNodeIdUnknown` — so clause 6.7's promise to a Part 10 client did not hold. - `DisposeAsync` disposed primitives out from under in-flight executor code; re-using a terminated `IntentId` destroyed the retained result and duplicated a NodeId; neither side registered the Robot Intent encodeables, so `Pose3DDataType` did not decode; host startup was hooked to a task that ran in only one hosting shape. - The result was published **after** the state went terminal; capabilities were resolved against a stale namespace table; an empty `ContentFilter` arrives as an empty element array rather than a null filter, so testing only for null made every unconditional mission transition silently untaken — a mission would run its first step and stop. ### Found by a later test-quality audit, after the above was written An audit of the test suite itself found five tests whose **names** asserted a normative requirement while their **code** asserted something weaker or nothing at all. The worst **fabricated the answer it checked**: a helper caught a decode failure and invented "5 supported intents, 6 axes, all facets true", so six capability-honesty assertions across four tests could pass while reading nothing from the server. The fallback is gone and the suite passes with honest reads. Chasing the facet gaps it reported led to the clause 12 defect above. Two further defects followed from completing that work: - **`RI-Interop-40010` was never computed**, and the code documented the omission as impossible to fix "because the calculator receives only a Robot Intent controller, not the linked OPC 40010 model". That was wrong: Annex B's structural requirement is an **inverse** `HasIntentController` reference, reachable from `IntentControllerState` — and the integration suite was already browsing exactly that. A server implementing Annex B could not claim Annex B. - **The published claim could go stale.** `SupportedFacets` was a snapshot taken during `RegisterAsync`, so a reference attached afterwards left it wrong. It is now bound through `BindRead` and recomputed on read, so the claim tracks the address space by construction rather than by discipline — which matters for anyone bolting Robot Intent onto an existing OPC 40010 node manager, where the attach ordering is often not theirs to choose. Two tests that **could not fail** were also replaced: one asserted `AccessLevel` equals `CurrentRead` to pin a read-only marking, but `BaseVariableState` initialises every variable to exactly that, so deleting the marking left it green; it is now a write expecting `BadNotWritable`, including a real OPC UA `Write` service call over a live session. Finally, a build break that a plain build hides: `RoboticsAotTests` used `out string?` in a project declaring `<Nullable>disable</Nullable>`. Building the project **without** `-f` suppresses `CS8632` through `NoWarn=nullable` and succeeds; building it **with** `-f net10.0` resolves `NoWarn` differently, `CS8632` fires, and `TreatWarningsAsErrors` makes it an error. CI builds per-TFM, so this would have failed there while passing every local build to that point. ## Testing and coverage | | | |---|---| | `dotnet build UA.slnx -t:Rebuild` | **0 errors, 2 warnings** — both pre-existing `CA1861` in `tests/Opc.Ua.Tools.Tests`, a file this PR does not touch | | **Whole `UA.slnx` test suite, net10.0** | **50 of 52 assemblies fully green** (measured at `40d9cf213`, before the `master` merge; the other two are explained below) | | `Opc.Ua.Robotics.Tests` | **467** net10.0, **447** net48 | | `Opc.Ua.Robotics.Intent.Tests` (live client <-> server) | **20** on net10.0 and net48, run repeatedly | | `Opc.Ua.OpenUsd.Tests` | **904** after merging `master` (664 before; the rest arrive with #4160 / #4162 / #4180) | | `Opc.Ua.Aot.Tests` | **121/121** | | `Opc.Ua.Di.Tests` (regression from the sample move) | **364/365**, one pre-existing skip | | **Changed-line coverage** | **90.7 %**, no file below 80 % — comfortably above the enforced band, which after #4158 applies only to patches larger than 100 changed lines | The two non-green assemblies are **not** caused by this branch. Rather than assert that, I ran the same selections at the merge-base `ae241b662` on the same machine: - `ServiceCallReassemblerTests.PrivateRequestSummaryFormatsKnownRequestKinds` expects `12.5` and gets `12,5` — a culture-sensitive assertion failing on a European locale. Fails 4/4 on **both** commits. - `CertRotationLiveTests.L2Cert2RotateCertificateDuringServerRestartRecoversSharedChannelAsync` fails on a teardown count. Fails 4/4 on **both** commits. - A third failure seen once, AOT `FindServersAsync` with `BadSecureChannelClosed`, did not reproduce: 4/4 on retest and 121/121 in the final run. Environmental. Two things are worth calling out because they would otherwise be invisible: - `tools/**` is **not** in the `coverage-thresholds.json` ignore list, and no test project referenced the OpenUSD tools assemblies — so their changed lines counted as uncovered and the real patch figure was 75 %, exactly the blocking threshold. Rather than widening the ignore list, the pure decisions in the viewport host (pointer-to-physical-pixel conversion, `RenderPickRequest` construction, stale-retry, backend discovery, pick-mode selection) were separated from the Avalonia/native shell and tested headless against a fake picking backend. - `dotnet build UA.slnx` **hides warnings on unchanged projects**; only `-t:Rebuild` surfaces them. That is how a `CA2263` introduced by a net48 portability fix survived several "zero warning" builds. The multi-target requirement earned its keep repeatedly: it caught a default interface implementation, `ArgumentNullException.ThrowIfNull`, `string.Create` with an interpolated handler, `ValueTask.FromResult`/`CompletedTask`, `Task.WaitAsync`, generic `Enum.GetValues` and generic `Enum.IsDefined`, and a non-generic `TaskCompletionSource` — none of which the net10.0 build complained about. A net10.0-**only** hang in the simulated executor and two load-sensitive flaky tests were also found and fixed at the cause. ### Merged with `master` `master` (through `e73e71184`) is merged in as `08205d81d`. Four conflicts, all where the two branches touched the same robot and OpenUSD surfaces, and none resolved by dropping a side: - #4162 added six files to `samples/MinimalRobotServer/` while this branch moved that directory to `samples/Robotics/MinimalRobotServer/`. They are kept at the moved path, and `UA.slnx` takes the union of both sides' new projects. - #4160 replaced `object` with a `UsdValue` union and rewrote `Flatten`; this branch had reformatted the same method and fixed a path in its doc comment. Git produced a hybrid that did not compile. The resolution keeps `master`'s `UsdValue` arm and `Flatten(UsdValue)` overload in this branch's block-bodied form, with the path fix retained. - `master` inserted `FindStageCamera` directly before two methods this branch had relocated, so the merge duplicated `WriteStageUsda` and `GetPrivateStateRoot`. Only `FindStageCamera` is genuinely new — `master`'s diff shows the other two as unmodified context — so the duplicates are dropped and this branch's `internal` versions kept, including the `GetPrivateStateRoot(string?)` overload the connector tests use to stay out of the real user profile. - `master` moved the OpenUSD stack to `0.4.0-alpha`; this branch pins `OpenUsd.Rendering` in the test project so the renderer-pick tests can reach the picking backend. Left at `0.1.0-alpha` that pin is below what the viewer now resolves transitively, which is `CS1705`. The pin follows `master`. CI was green on `baaf35e9` with **190 checks passing and none failing**, across every build leg (net472 / net48 / netstandard2.0 / netstandard2.1 / net8.0 / net9.0 / net10.0, Debug and Release) and the full test matrix on Windows and Linux. Since then `master` has been merged twice more and two CI-stability fixes landed (see below), so the current head is re-running. Locally, `dotnet build UA.slnx -t:Rebuild` gives **0 errors** and the 2 pre-existing `CA1861` warnings; Robotics **467** net10.0 / **447** net48, Robot Intent **20** on both, OpenUSD **904**, AOT **121/121**, source generation **3771**. ### Review round Five reviewers over disjoint areas found defects worth calling out, all fixed in `3ffec2c1`: - **A refused mission step was reported `Succeeded`.** The advance path returned the same `false` for "steps exhausted" and "step refused", so a protective stop between steps told a supervisory system the mission had completed while the robot stood halted with work unexecuted. - **Three attacker-controlled NodeIds reached the executor unvalidated** — `Pick.Tool`, `Place.Tool`, `ToolChange.DockStation`. `DockStation` drives a physical tool-exchange motion to a location the server never authorised. The replacement test enumerates NodeId members by reflection so the next one added cannot be missed. - **A lock-order inversion that loses a stop.** `CanCancel` and the mission condition evaluator ran under the host lock while the executor's own thread re-entered it via `ReportBlendBegin`. - **A non-terminal executor outcome killed the pump** from outside the `try/catch` that exists because the executor is untrusted — `Ready` stayed true and submissions kept being accepted while nothing moved. - **`Pause` published `Suspended` while the robot kept moving.** Now queue-only and honest about it; the specification gained the matching rule ([opcua-drafts `357a558`](https://github.com/marcschier/opcua-drafts/pull/47)). - **`RI-RealTimeChannel` and `RI-Safety` were false claims** — granted on an empty folder with no methods, and on the mere presence of a Mandatory node. - **The Part 10 promotions never promoted.** Namespace-1 BrowseNames declared second members beside the inherited ones; the conformance test could not see it because its lookup stripped the prefix. Fixed upstream and regenerated. - **Renderer picking never worked.** The reflective probe bound a renderer that is not a picking backend and throws off its owner thread. It now binds only `IRenderPickingBackend`, falls back immediately and loudly, and the docs say plainly that renderer picking is unavailable with this package version. - **The client lost command authority the instant it gained it** — monitored items report the current value first, and the lease treated every notification as a loss. ### CI stability fixes for master-side failures The CI split in #4158 turned on legs that had never run, and two of the failures they exposed were real bugs on `master` rather than anything this branch introduced. Both are fixed here because they were blocking every branch: - **ECC key generation was broken on macOS.** `ExportExplicitParameters` throws `PlatformNotSupportedException` there — Apple's SecurityTransforms supports named curves only — and it was called for exactly one value, the curve order `n`. The four curves OPC 10000-12 admits have published orders, so those are carried directly, with `CurveOrderTableMatchesThePlatform` checking each against the platform's own exported order wherever export works, so the constants cannot silently drift. This accounts for the two ECC failures in `test-macOS-latest-Server`. - **The certificate-leak detector raced asynchronous disposal.** It polled a fixed budget and reported whatever it saw, which cannot distinguish a genuine leak (count steady) from a suite still draining fire-and-forget disposals (count falling) — on a loaded agent that produced `Certificate leak detected: 2 instance(s)` with every test passing. It now waits for the count to **settle**, so a real leak is reported sooner than before while a slow drain gets as long as it keeps progressing. Shared by every suite that asserts no certificate leaks. The remaining macOS failures on `master` (`Gds`, `Tools`, `Redundancy.Samples`, `aot-macos`) have different root causes, are not reproducible without a macOS host, and are **not** addressed here. ### Merged with `master` again `master` through `416d619c4` (#4158, the CI matrix split and graduated coverage bands) is merged in as `08ae4528`. One conflict, in `docs/DeveloperGuide.md`, where both sides had deliberately rewritten the coverage-gate section: - `master`'s rewrite is kept in full — graduated `patch.bands`, Codecov being informational-only, and where the numbers surface per CI system. - This branch's paragraph about the `ignore` globs is kept too, re-sited ahead of those subsections. I re-checked it against the merged `coverage-thresholds.json` rather than assuming it still held: the list is still `tests/**`, `samples/**`, `obj`, `bin`, `*.g.cs`, with **`tools/**` absent** — so the warning that anything under `tools/` is measured like product code, and that an assembly no test project references contributes changed lines counted as *uncovered*, is still accurate and worth keeping. - `master`'s "reproduce a coverage failure locally" line supersedes this branch's near-identical one, so only `master`'s survives. Post-merge: `dotnet build UA.slnx -t:Rebuild` **0 errors**, 2 pre-existing `CA1861`; Robotics **467** net10.0 / **447** net48; Robot Intent **20** on both. ### Getting CI green The pipeline had never actually run on this branch until now (it was waiting on the fork-PR approval gate), so the first run surfaced four real problems, each fixed at source: - **Seven build legs failed** because `IntentEnabledRobot` pins `net10.0` for the OpenUSD payload but never opted into `RestrictForLegacyTfm`. On the legacy legs the libraries build as net472/net48, where `Opc.Ua.Types` exposes the `System.Threading.Lock` polyfill publicly, and that collides with the sample's own net10.0 BCL (`CS0433`). `Opc.Ua.OpenUsd.Connector.Viewer` was already using the opt-out for the same reason. - **A Linux-only test failure** turned out to be a real defect, not a platform quirk: `SetPose` published the *commanded* interpolated pose beside joint frames derived from the *solved* joints, so the two disagreed by the inverse-kinematics residual — about 8 mm, constant after any real motion. That is the lying-digital-twin defect one level deeper than the per-step IK fix reached. `SetPose` now derives both from the joint angles. - **Three source-generator tests** asserted the old emitted shape. The clause 6.2 Null-Variant fix was implemented in `ObjectTypeProxyGenerator` so every generated proxy benefits, which changed that shape; the tests are updated and strengthened to pin the `IsNull` branch as well. - **A CI-only timeout** in the simulated arm drain: the executor runs on a manual clock so the tests do not depend on machine speed, but the drain guarded itself with a wall-clock deadline, which put that dependence back. CI runs with coverage instrumentation, which is enough to take a per-step IK probe from ~0.6 s to over 5 s. The guard is now a simulated step budget. One failure was **not** from this PR and did not recur: a Roslyn `AccessViolationException` compiling `Opc.Ua.ISA95` on one leg, while the Debug leg of the same target passed on identical code and the Release build succeeds locally. ## Related Issues - Relates to #3827 — the request that prompted the robotics intent work. - Builds on #4127. The companion specification is drafted in the open at [marcschier/opcua-drafts#47](https://github.com/marcschier/opcua-drafts/pull/47) (`metaverse-specs/robot-intent/`), with the prior art and the reasoning behind each decision in the research document beside it. **Nothing in it is normative or endorsed by the OPC Foundation, and its NodeIds and namespace URI are provisional** — which is the main reason this is a draft. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] 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. On the test checkbox, precisely: the whole `UA.slnx` suite now runs on net10.0 with **50 of 52 assemblies fully green**, and `Opc.Ua.Robotics.Tests` passes on **all five** target frameworks including net472, net8.0 and net9.0 which had never previously been exercised. The two remaining failures are reproduced identically at the merge-base and are detailed above, so I have ticked the box; if you would rather it stayed unticked until those two are green, say so and I will revert it. ## Points I would most like feedback on 1. **The namespace and NodeIds are provisional.** They come from a draft specification, not from the OPC Foundation. If this lands before the model is registered, the identifiers will move. 2. **The model lives in `Opc.Ua.Robotics`.** It is standalone on the base UA namespace and takes no dependency on OPC 40010, so a separate `Opc.Ua.RobotIntent` package would arguably be cleaner. Keeping it here was a deliberate call — this is where the robot-facing API already is — but it is the easiest thing to change now and the hardest later. 3. **Removing the convention API from #4127 is a breaking change** for anyone who adopted it. It was explicitly marked non-normative, so I have assumed that is acceptable — please say if a deprecation period is wanted instead. 4. **Serial execution** is a deliberate simplification. Concurrent `None`/`Soft` intents would be an optimisation, not a correction, but it is worth agreeing that reading. 5. **`samples/Robotics/` reorganisation** moves `MinimalRobotServer`. Happy to split that into its own commit if it makes review easier. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1dab1302-19c5-4a9b-a50c-d97d389713aa Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b	2026-08-08 09:08:40 +02:00
Marc SchierandCopilot	ca89ac85a7	Add a Generators companion-spec sample and compose servers at a site level (#4180) ... # Description Adds a **Generators companion-specification sample** alongside the existing pump sample, and a **`SiteCompositionServer`** that composes several device servers into one OpenUSD scene at a supervisory level — demonstrating that servers can be aggregated without mirroring their address spaces. Three things, in dependency order: ### 1. `GeneratorServer` A self-contained server realising the draft Generators spec (`http://opcfoundation.org/UA/Generators/`) end to end: N simulated generating sets, DI + Machinery integration, a twelve-state operating state machine, four protection alarms per set, six control methods, and a per-set OpenUSD twin. The organising idea is that **load fraction is the only independent variable**. Everything else is a function of it: ``` V̇_f(x) = 3.67 + 100·x fuel rate [L/h] η(x) = P(x) / (V̇_f(x) · ρ · LHV) efficiency S = P / PF I = S / (√3·V_LL) f = N·p / 120 ``` This is not stylistic. When each measurement is an independent oscillator — as the pump sample's simulation once was — a server happily publishes a duty point no real machine could occupy, and no test can catch it because there is nothing for the values to be inconsistent *with*. Deriving them from one variable means `P = √3·V·I·PF` and `η = P/(V̇·ρ·LHV)` hold at every tick by construction. The published `DATASHEET.md`, the engineering ranges, the trip points and the simulation all read the same constants in `GeneratorDatasheet.cs`, so the document and the server cannot describe different machines. The machine is a fictitious **SimGen Systems GenX-500** (400 kW prime / 440 kW standby, 400/230 V, 50 Hz, four-pole). Figures are representative of a real industrial genset class; no vendor is named anywhere in the code or docs. ### 2. Cross-server composition in the connector `RemoteSessionFactory` already existed in the client library but the connector CLI never set it, so cross-server composition was unreachable. `--federate` wires it up: the connector opens a session to each server named by a component binding, discovers its representations and drives its bindings into the same stage. It is **opt-in** because the endpoint the connector dials comes from the server being rendered rather than from the operator, which makes honouring it a trust decision. Federation is best-effort per component — a subordinate that is down is logged and skipped, and the rest of the scene still renders — which is the only `src/` change in this PR. ### 3. `SiteCompositionServer` Owns no devices. It publishes a site stage plus one cross-server component binding per subordinate, carrying that server's `ComponentServerUri` and `ComponentEndpointUrl`. Nothing is mirrored, so there is no cache to invalidate and no second copy of the truth. ## Notes for reviewers **Everything interesting in this PR was found by running the servers, not by reading the code.** Six defects, none of which looked wrong in review: - `IsShutdown` and `SubsystemName` are *optional* on `GeneratorProtectionAlarmType`, so the generated factory does not materialise them. `CreateOrReplace` alone produces a child that exists, appears in `GetChildren` and holds the right value — but carries **no `ReferenceTypeId`**, so no browse can reach it. Every alarm was publishing its trip without saying whether the trip stops the machine or which subsystem to go to. `AddXxx(context)` first is what gives the child its `HasProperty` reference. - **The protections could never fire.** Every trip point sits outside the band the datasheet curves produce — that is what a datasheet *means* — so all four alarms, the shutdown class, `ResetFaults` and the whole `Fault` branch of the state machine were unreachable while the README documented them as observable. Overload was worse: the load clamp ceiling was set to exactly the trip point, making a strictly-greater-than comparison unsatisfiable. Faults are now injectable, and the first set — the one the hero camera frames — develops one on a slow rotation. - **A shutdown trip removes the condition that caused it**, because oil pressure and coolant temperature are only supervised while the engine turns. Each alarm went active for one tick and cleared, leaving an operator with a stopped machine and no indication of why. Shutdown-class alarms now latch until the set leaves the shutdown state. Every unit test that checked the *condition* passed throughout — the defect only appears when the trip and the supervision interact over time. - **`Synchronizing` and `Paralleled` were declared, drawn in the README and never entered.** The first set now energises a dead bus and closes onto it while every other set synchronises to a live one — both how a real plant parallels, and what makes the two states observable. - **The tick raced client method calls.** The simulation tick runs on a thread-pool thread while method calls arrive on request threads, and both transition sets and write the same nodes. A tick moving a set to `Cooldown` and a concurrent `EmergencyStop` could interleave the paired `CurrentState` / `CurrentState.Id` writes and leave a client with a state *name* from one transition and a state *node* from the other — the exact failure the paired write exists to prevent, reintroduced by the threading model. Both paths now take one gate. - Low oil pressure supervised from raw speed **tripped every set during cranking**, because pressure has not built yet. Now gated on `IsSpinning`, which is what a real start-up bypass does. Smaller fixes: shutdown trips are applied after the whole evaluation pass (stopping mid-loop made the remaining conditions read healthy, collapsing simultaneous trips to whichever came first in the table); `ResetFaults` reports its clears as events (a client learns of condition state changes only through events, so a silent clear leaves an alarm-list client showing it forever); the start counter moved into the transition so commanded starts are counted; the federated connector closes the remote session if the connector constructor throws between taking ownership and registering it; and `prepare_machinery_nodeset.py` now *asserts* the IA namespace is last before removing it rather than only claiming so in its docstring — removing any earlier entry would silently renumber every namespace after it and rebind their NodeIds. **A model limitation is left visible rather than papered over.** `GeneratorStateMachineType` declares an emergency stop only out of `Running`, `Loaded` and `Paralleled`, so the sample refuses `EmergencyStop` from `Starting` and `Warmup`. A real panel stops from anywhere. That is the specification's shape, not this sample's choice, so it is pinned by a test and named in the docs. **`Model/` vendors a reduced Machinery nodeset**, derived mechanically by whitelist. The full official nodeset does not survive the model source generator (`MODELGEN003`) and drags in IA through a single optional `Stacklight` member a generating set does not have. Deriving it by script keeps the provenance checkable; the whitelist is the only thing to edit when more types are needed. **Never hand-edit the generated output.** **The federated scene renders end to end.** Getting there surfaced three more defects, all of which produced a plausible-looking scene with nothing in it: the generator sample's plant aggregation was created under an already-registered `DeviceSet` and so was invisible to every client (it had *never* rendered geometry, standalone or federated); the connector's asset fetch ran on the primary session only, so composition referenced layers that were never downloaded; and both generator colour bindings wrote `primvars:displayColor` to prims that did not declare it, so the renderer rejected every update while the file still looked correct. Cross-server components compose under the subordinate's own root, so the site layer now *places* `/Plant` and `/Powerhouse` rather than leaving them stacked on the origin. **`OpenUsdRepresentation` is now mounted with `HasAddIn`, not `HasComponent`** (review feedback). The nodeset says so explicitly — *"Mounted with HasAddIn"* — but every server in the tree used plain `HasComponent`. It survived because `HasAddIn` is a *subtype* of `HasComponent`: the representation still browses, still aggregates and still drives a twin, so every functional test passed before the fix and after it, and only a conformance checker could tell. Seven mount sites corrected. `PumpDeviceIntegrationServer` and `MinimalRobotServer` had hand-rolled the mount instead of calling the shared `CreateRepresentation` helper, which is exactly how they drifted from it. Both sample E2E suites now assert the reference type across **every** discovered representation — a narrower check would have missed the plant-level representation that arrived later from master still carrying `HasComponent`. **Live colour is not achievable in the current OpenUSD viewer, and that is now documented rather than worked around.** Proven with a standalone probe: `primvars:displayColor` resolves as `color3f[]` from the `UsdGeomGprim` schema whatever the layer declares, and the `OpenUsd 0.4.0-alpha` managed API has no writer for that type. Bound `UsdPreviewSurface` materials are not shaded either, so geometry renders grey unless it also carries an explicit `displayColor` primvar. The **visibility** bindings carry no such caveat and are what the samples rely on to show state. Filed upstream as [openusd-dotnet#2](https://github.com/marcschier/openusd-dotnet/issues/2), [#3](https://github.com/marcschier/openusd-dotnet/issues/3) and [#4](https://github.com/marcschier/openusd-dotnet/issues/4), and linked from `docs/OpenUsd.md` so a reader can check whether they have since been fixed. ## Related Issues No tracking issue — this is additive sample and documentation work with a single small, self-contained change to `src/` (per-component error isolation and session-ownership hardening in `OpenUsdConnector.Composition.cs`). Happy to open one if maintainers would prefer it tracked. ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] 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. ### Testing The generator fixtures live in `tests/Opc.Ua.OpenUsd.Tests/Generator` — **82 tests**, no new project. `Opc.Ua.Di.Tests` would have been the natural home next to the pump fixtures, but both device samples source-generate their own reduced `Opc.Ua.Machinery` model and expose it via `InternalsVisibleTo`, so referencing both from one assembly makes every Machinery type ambiguous (CS0433); `Opc.Ua.OpenUsd.Tests` references no other sample and already hosts `RobotAssetContractTests`. The suite runs **870** on net10.0 and net48 (784 existing + 86) and **788** on net472, where the sample does not exist and only the framework-independent asset-contract tests run — forgetting exactly that exclusion broke CI on the sibling pump PR. The tests target the things in this sample that fail *quietly*: - **Datasheet conformance** holds the model to its own claim — rated speed is `120·f/p`, rated current follows from the rating, the fuel curve reproduces the published table, and `η = P/(V̇·ρ·LHV)` reconciles at every load. Efficiency is swept across the whole simulated range to show it stays inside (0,100) and rises monotonically; a fit that goes negative somewhere in its range is a coincidence that happens to look right at the duty point, not a model. A further test reads `DATASHEET.md` and checks the document quotes the figures the server actually serves. - **Drift between the two descriptions of the state machine** — the physics' `IsLegalTransition` and the model's `GeneratorStateMap` — is checked in *both* directions. A transition the physics permits but the map lacks moves a machine without telling a client; one the map holds but the physics refuses is dead weight that looks supported. - **Reachability at run time**, not just in the declared table — the test that catches states the model declares and nothing ever enters. - **That every protection can actually fire**, which is the half of the alarm contract that "a healthy set annunciates nothing" does not cover. A **hosted end-to-end fixture** connects a real client and asserts what the address space actually exposes — the aggregation at `/Powerhouse/Generators`, its component binding, and every per-set twin. That is the only kind of test that catches the registration defect above: the node object had the right browse name, binding and asset reference, so every test that inspected objects passed while no client could see it. **Every state-machine and protection fix was checked by mutation.** Restoring the clamp ceiling, the direct `Running → Loaded` path, the old start-counter placement, or removing the alarm opt-in each makes the corresponding test fail, naming the specific defect. A test that has never been seen to fail has not been shown to work. Verified against a running plant: `Synchronizing`, `Paralleled` and `Fault` all observed on a two-set server, with `HighCoolantTemperatureAlarm` seen active on the fault subject. Method behaviour verified separately — per-set distinct `CurrentState` nodes, `EmergencyStop` → `EmergencyStopped` with `Start` then refused as `BadInvalidState`, `ResetFaults` → `Off`, `SetOperatingMode(9999)` refused with the mode unchanged, and the other sets untouched throughout. Also verified as a three-server federated stage. 0 warnings on a clean Release rebuild of both samples, the client library, the connector and the tests, across every TFM. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe608993-f3c9-4779-a6b9-9a9eabfc24ba	2026-08-07 10:48:02 +02:00
Marc SchierandCopilot	81d726b7ad	Coordinate the robot cell, and make the twin show what it is actually doing (#4162) ... # Description `MinimalRobotServer` drove its two mobile manipulators through each other. The platforms *did* clear — R1 and R2 trace figure-eights 1.2 m apart at closest approach, exactly as `README.md` claimed — but each **arm reaches 1.611 m** (link offsets `0.260 + 0.680 + 0.670 + 0.158` from `robot.usda`), so the arms swept straight through one another while the chassis politely missed. Clearance has to be argued about the deployed arm, not the footprint. Three related gaps sat alongside it: motion was open-loop (two independent sine paths with no awareness of each other), only R1 had a gripper, and nothing was ever actually picked — `PartA`/`PartB` were static children of a single table and the arm replayed a canned eight-pose loop unrelated to where either the robot or the table was. This replaces all of that with a coordinated transfer cycle. ## Keeping them apart The cell is divided into reservable zones, and a robot reserves the zone *immediately ahead* before entering it, releasing the one behind once clear — block signalling rather than a global schedule: | Zone | Extent | Occupancy | | --- | --- | --- | | `EndZoneA` / `EndZoneB` | \|x\| > 1.3 m | one robot — a deployed arm needs the whole end | | `CorridorEastbound` | \|x\| ≤ 1.3 m, northern lane (y = 0.35) | one robot | | `CorridorWestbound` | \|x\| ≤ 1.3 m, southern lane (y = −1.15) | one robot | | `DockA` / `DockB` | around each charging dock (y = −2.0) | one robot | Three rules follow: - **Opposite-direction traffic passes**, being in lanes 1.5 m apart; same-direction traffic queues rather than overtaking. - **An arm may only leave its transport envelope inside an exclusively reserved end zone.** This is the rule that actually fixes the reported defect. - **A robot with no work parks on its dock**, releasing the end zone so the other robot can deliver into the station it serves. That last rule is not cosmetic. Without it the cell deadlocks permanently on the first handover: an idle robot standing beside its own table blocks the very delivery it is waiting for. A state trace showed R1 blocked at `x = 0.98` forever while R2 sat idle holding `EndZoneB`. The docks are in a southern layby rather than beside the stations, because a robot parked next to a station is close enough to foul one turning into it. ## Where the part actually is A carried part's pose is computed by forward kinematics (`RobotKinematics`) from **the same six axis values the server publishes**, not from the choreography that produced them — if the arm and the part were computed independently they would visibly drift apart. `RobotArmSolver` solves shoulder and elbow so the tool points down onto a slot, and is checked against that same forward kinematics, landing the tool on every slot of both tables to within 1e-6 m. ## Also simulated Trapezoidal accel/cruise/decel travel with the heading following the path; speed-and-separation slowdown near the other robot; an emergency stop that now halts motion rather than only blinking the beacon; battery drain with a charging detour; seeded grip-slip faults with floor recovery; and KPIs (parts moved, cycle count, last/average cycle time, utilisation, fault count). Both robots now carry a gripper. ## Two hidden couplings this surfaced - `m_r1NodeId` was keyed on *"has a tool"*, so giving R2 a gripper silently moved the dynamic-tool demo and the speed-override command target onto R2. Now keyed on R1 explicitly. - `RobotPositionsDriveLiveOpenUsdSceneAsync` sampled a fixed 1.2 s window and assumed the robot was always moving. Under a duty cycle a robot legitimately stands still while gripping, dwelling or charging, so it now polls until movement is observed — the assertion's intent is that live positions reach the scene, not that the robot never stops. ## Invariants are asserted, not assumed `CellChoreographyTests` runs ten minutes of simulated time at 50 ms and checks on **every step** that: - oriented footprints never overlap (an oriented box, not a bounding circle — the circle is so pessimistic for a 1.15 × 0.60 m platform that it demands clearances no real cell would need); - the two tool centre points never meet; - a zone is never doubly occupied; - arms only deploy inside an end zone; - parts are conserved — never duplicated, lost or carried twice; - a carried part sits exactly on the tool centre point. Plus the round trip completing, KPIs accumulating, the emergency stop halting every robot, dropped parts being recovered, and the battery draining and recharging. The README documents the zoning, traffic rules and cycle, **corrects the misleading clearance claim**, and lists ten further realism extensions that were deliberately not built (ISO 10218 / TS 15066 speed-and-separation monitoring, scanner cones, tool changer, inspection station, conveyor infeed, gate interlock, torque telemetry, maintenance counters, time-sampled pose history, traffic priority). ## Making the cycle visible Running the sample end to end against the OpenUSD connector showed the robots driving the cycle and the arms reaching the slots while **nothing was picked up or moved**. The choreographer tracked part poses and gripper opening purely in memory: the address space only ever exposed the platform poses and the twelve axis angles, so a connector had nothing to subscribe to for the parts and the blocks stayed wherever the asset authored them. A `CellTwin` folder now holds a `ThreeDCartesianCoordinates` position and a `ThreeDOrientation` heading per part, plus a jaw position per robot, bound to `/Cell/Parts/PartNN` and to the gripper jaws. The variables are registered before the motion device system is built - a binding can only carry a `SourceNodeId` that already exists - and sit outside the robotics subtree, because a workpiece is not a device and the robotics builder validates every node below its root as one it allocated itself. **The poses are published as structured vectors, not as a matrix.** A viewer composes a prim's matrix from the vector translate and rotate ops; the `matrix4d` profile is explicitly left unresolved by the reference connector, which drops a binding it cannot convert *in silence*. That is the shape the first attempt took, and it failed without a word in any log. The jaws needed the asset restructured. A carrier cube and a finger mesh were siblings under the gripper body, so driving the carrier would have slid it out from under the finger it is bolted to. Each jaw is now one `Xform` owning both, declaring the single matrix op every live-bound prim declares, with the finger points rebased onto the jaw origin. Closed puts the finger faces on the 35 mm half-width of the block, so the gripper grips it rather than passing through it. `RobotAssetContractTests` now holds **every** live-bound prim to the single-matrix contract, not just the two robot mount points. ## No part starves Collecting the lowest free slot meant a part that never landed in slot 0 was never collected at all: with two parts seeded on `WorkTableA` the second sat untouched for the life of the process while the robots shuttled the other one past it - which reads in the twin as the robots ignoring a block sitting in front of them. Parts are now stamped with the order in which they came to rest and the one that has waited longest is collected, which is what a real cell does with a buffer anyway. Verified end to end: all three parts circulate between both tables, the heading follows the carrying robot and resets on placement, and the jaws stroke between open (0.060) and closed (0.047) as parts are gripped and released. The OpenUSD packages move to 0.3.0-alpha. ## The part was not in the gripper Running it end to end showed a workpiece floating clear of the jaws that were supposed to be holding it. The platform pose and the workpiece pose leave the server as **separate bindings published by different loops**: positioning at 5 Hz (`UpdateIntervalMilliseconds = 200`), the simulation tick at 20 Hz. A carried part rides the tool centre point, so it was being drawn against a platform up to four ticks stale. Measured against the live scene with the arm stowed — where the part-to-platform distance is fixed by construction — the part sat anywhere between **0.65 m and 0.90 m** from its platform instead of at a single distance. A quarter of a metre of drift, three part widths, in front of a 96 mm jaw opening. The platform now publishes at the tick rate, and a carried part is derived from the pose that was **actually written to the address space**, recorded at the `SetFrameValue` call site rather than where the sample is produced. Each value was individually correct the whole time; only their relationship was wrong, which is why nothing caught it. `RobotOpenUsdE2eTests` now rebuilds the tool centre point from the values authored into the scene and asserts a carried part sits on it. It reproduced the defect at 0.49 m before the fix. The tolerance is the sampling straddle of reading two independently delivered bindings — one publish period is 43 mm at cruise — leaving an order of magnitude between it and the defect. Two assertions in the positioning test wanted the robots a metre apart **in X**. They pass each other in opposite corridor lanes, where X is all but identical and the separation is entirely in Y; and before the first sample arrives both platforms sit on the zero frame and coincide exactly. They now measure separation in the plane, once both have been seen to move — this was failing intermittently on net48 already. ## The opening view Framing the bounds of an enclosed scene automatically puts the eye *inside* it: the viewport opened on a close-up of whichever robot happened to be nearest, looking through the fence. The cell now authors `/Cell/OverviewCamera` — the whole floor, both work tables, the docks and the controller cabinet, from 12 m out at 28 degrees of elevation — and the connector opens on the first camera the served root layer declares. It already had the plumbing to honour a camera path; it was simply never given one. `--camera` still overrides the choice outright, and `RobotAssetContractTests` asserts the establishing shot is ordered before `/Cell/TopDownCamera`. ## The mounted gripper was never reported `DynamicToolIsComposedAsync` was failing on both TFMs, in isolation, and a longer timeout did not help. It predates this branch — I confirmed it at `58fd5e8ca` with a clean worktree. It turned out not to be a flaky test at all. **Part 5 §9.32.2** lets only a node carrying a `NodeVersion` property trigger a `ModelChangeEvent`, and `AsyncCustomNodeManager` drops entries for nodes that lack one. Neither the mounted tool nor the robot it hangs off had a `NodeVersion`, so **every addition was filtered out**. Removal kept reporting, because a deleted node is no longer in the manager's index and so takes the *"not mine, pass it through"* branch of the same filter. That asymmetry is what made it look like flakiness. A connector starting while the tool was mounted composed it and could still deactivate it later; one starting in the six-second gap was never told the tool appeared and never composed it **at all, for the life of the process**. Which happened depended only on the phase the fixture reached the test in — near-constant, hence a consistent failure rather than an intermittent one. Measured directly before the fix: model-change events arrived only on detach, exactly 18 s apart, and every resolve returned zero components: ``` 14:00:12.738 SERVER mounted <- no event 14:00:24.752 SERVER detached 14:00:24.801 model-change event <- only this ``` After enabling model-change tracking on the robot, the prim tracks the cycle — `A` = composed active (12 s mounted), `i` = inactive (6 s detached): ``` ....AAAAAAAAAAAAAAAAAAAAAAAiiiiiiiiiiiiAAAAAAAAAAAAAAAAAAAAAAAAiiiiiiiiiiii ``` **The test now asserts a transition**, not a momentary state: the prim seen both active and inactive across more than one full cycle. Unlike the old check this cannot pass on a connector that only ever hears about one direction — which is exactly what the previous pair of tests could not distinguish, since the sibling `DynamicToolIsDeactivatedWhenDetachedAsync` was satisfied by the *initial* compose and never exercised reconciliation at all. Two robustness gaps in the connector fell out of this: a rejected model-change monitored item was reported as success, leaving every `Dynamic` component frozen at whatever it resolved at start-up; and the initial compose ran *before* the subscription existed, so a change in between was seen by neither. The first now fails loudly, the second re-resolves once after subscribing. `Opc.Ua.Di.Tests` is now **402/403 on net48** (was 3 failures) and on net10.0, across three consecutive runs of the dynamic-tool tests on each. ## Related Issues - Fixes #4161 ## Checklist - [x] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [x] 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. Verification: full `dotnet build UA.slnx -m:1 /p:UseSharedCompilation=false` — 0 errors, no new warnings. `Opc.Ua.Di.Tests` 399/400 on **both net10.0 and net48** (1 pre-existing skip), `Opc.Ua.OpenUsd.Tests` 635, `Opc.Ua.Robotics.Tests` 103. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485	2026-08-04 06:58:29 +00:00
Marc SchierandCopilot	fb160a8f39	OpenUSD: the OPC UA to OpenUSD binding, scene materialization and connector (#4126) ... **Stack 4/5** — base #4125. Part of the #4096 split; see #4096 for the overall picture. > Review after #4123, #4124 and #4125. ## What this is The generic OPC UA to OpenUSD binding, scene materialization, and the connector that drives a live stage from a running server. ## Contents **Binding (Part 1)** — `Opc.Ua.OpenUsd` and `Opc.Ua.OpenUsd.Server`. A server describes how its Variables drive USD prims, and serves the USD layers it refers to as Part 5 files with a content digest. **Scene materialization (Part 2)** — `Opc.Ua.OpenUsdScene` and `Opc.Ua.OpenUsdScene.Server`, including the USDA writer. - **Escaping fix**: string values, composition-arc references, variant set names and selections, `doc` and prim names were all emitted unescaped, so a value carrying a quote or a newline could author its own layer syntax. Every emit site now goes through the escapers. The tests assert the right property — not that the payload is absent (it legitimately survives inside the escaped literal) but that no payload becomes its own line of layer syntax. **Client** — `Opc.Ua.OpenUsd.Client` streams live values into an `IUsdSink`. `CompositeUsdSink` fans out to several sinks; the file sink interpolates between server samples so motion is continuous rather than stepped at the publishing interval. **Connector** — `tools/Opc.Ua.OpenUsd.Connector` composes a stage from a live server. The optional in-process viewport lives in a separate assembly probed at run time, so the connector still runs — and says why — when it is absent. - **Fix**: the asset cache and PKI root moved off the shared temp directory to `LocalApplicationData`. **Sample** — `PumpDeviceIntegrationServer` gains an OpenUSD representation of the pump. ## Notes for the reviewer - The viewport assembly is net10.0-only. It is what exercises the `Directory.Build.targets` change from 1/5: without it, the `net8.0` leg of the per-TFM CI build fails with `CS0433` because the `System.Threading.Lock` polyfill collides with the real type. - `UsdAssetDelivery` hashes through `SHA256.Create` rather than the static `HashData`, which does not exist on the net472/net48 targets this test project also builds for. ## Validation `dotnet build UA.slnx` — 0 errors, 0 warnings. | Suite | Result | | --- | --- | | Opc.Ua.OpenUsd.Tests | 621 / 0 | | Opc.Ua.Di.Tests | 321 / 0 | --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485	2026-08-01 07:55:58 +02:00