UA-.NETStandard

mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-24 19:13:02 +08:00

Author	SHA1	Message	Date
Marc SchierandCopilot	6795073d2d	Mount OpenUSD representations with HasAddIn (#4239 ) # Description Mount every `OpenUsdRepresentation` AddIn with `HasAddIn` rather than plain `HasComponent`. - Correct the `IOpenUsdRepresentedType` placeholder in the OpenUSD NodeSet. - Route Pump, Minimal Robot, and Intent sample authoring through the shared `CreateRepresentation` helper. - Remove redundant representation subtree rebasing from sample call sites. - Add model, in-process sample, and over-the-wire regression coverage. - Update the OpenUSD documentation and sample diagrams. `HasAddIn` is a subtype of `HasComponent`, so the incorrect reference still browsed and rendered successfully but failed companion-model conformance. ## Related Issues No related issue. ## Validation - `Opc.Ua.OpenUsd.Tests`: 914 passed on .NET 10. - Pump and Minimal Robot sample builds completed without warnings. - Pump and Robot `RepresentationsAreMountedWithHasAddInAsync` E2E tests passed. ## 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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 19119d71-e1bc-41c9-ba0d-2d1dea437167	2026-08-12 11:31:55 +02:00
Alexander Köpke Marc Schier Marc Schier Copilot	0edfe237a6	Bound in-memory historian retention (#4234 ) # Description Bounds raw-data memory usage in `InMemoryHistorianProvider` with a configurable source-timestamp retention window. - Adds `RawDataRetentionPeriod`, defaulting to one hour; `TimeSpan.Zero` restores unbounded retention. - Keeps `MaxSamplesPerNode` as an additional hard cap, with both limits enforced. - Applies retention across bulk, best-effort, update-as-insert, and transactional write paths. - Refreshes the retained high-water timestamp after raw deletions so later out-of-order inserts use the current archive contents. - Explicitly keeps the ReferenceServer conformance historian unbounded because it seeds a multi-hour archive and tests arbitrary historical timestamps. - Documents the default and configuration in `HistoricalAccess.md` and the pump sample README. - Adds regression coverage for defaults, custom and unbounded windows, combined limits, bulk/atomic writes, out-of-order inserts, and both raw deletion APIs. ## Related Issues Addresses the multi-gigabyte memory growth observed when `PumpDeviceIntegrationServer` runs continuously with its in-memory historian enabled. ## Checklist - [x] I have signed the CLA and read the CONTRIBUTING guide. - [x] I have added tests that prove the fix is effective. - [x] I have added the necessary documentation. - [x] I have verified the targeted build and analyzer-enabled historian test suite complete without warnings. - [ ] I ran all tests locally using `UA.slnx` against .NET Framework and .NET 10. - [x] I have addressed all PR feedback received. --------- Co-authored-by: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@hotmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-08-11 08:14:10 +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	a6a5275006	Add a PumpX-2000 datasheet and diagrams to the pump sample (#4138 ) # Description The `PumpDeviceIntegrationServer` sample simulates OPC 40223 pumps but had no product documentation, a thin nameplate, and a duty point that was not physically self-consistent. This PR gives the sample an official-style product datasheet for its simulated asset and aligns the server to it, so the document and the running address space cannot drift apart. ### New: `samples/PumpDeviceIntegrationServer/DATASHEET.md` A vendor-style datasheet for the fictitious SimPump Corp PumpX-2000 (clearly marked as a simulated device): - Nameplate table mapped to OPC UA browse paths and namespaces (DI / Machinery / Pumps) - Best-efficiency duty point, characteristic curves and a performance table - Operating limits, motor / materials / dimensions data - Instrument list (tag → browse path → unit → `EURange`) and a Mermaid process schematic - Alarm trip points and the deterministic simulation profile ### Server aligned to the datasheet - `PumpDatasheet.cs` — the datasheet expressed as compile-time constants. - `MaterialiseNameplate` — materialises the 19 optional identification properties through the generator-emitted `AddXxx(context)` helpers, so each carries the browse name, namespace and DataType declared by the model (21 nameplate fields in total per pump). - `WithIdentification` now assigns the full datasheet nameplate to every simulated unit. Product-level fields are shared; unit-level fields (`SerialNumber`, `AssetId`, `ComponentName`, `Location`, `FabricationNumber`) are derived from the pump number, so `--pumps N` yields a consistent nameplate for each instance. - Simulation rebuilt around the datasheet curves. Volumetric flow is the only independent variable; differential pressure, mass flow, efficiency and shaft power are derived from `H(Q) = 32 − 0.0104·Q²`, `η(Q) = 72·(1 − 0.6·((Q−25)/25)²)` and `P = ρ·g·Q·H/η`, so `P = Δp·Q/η` holds at every tick. Previously every measurement was an independent sine and the sample published 5 kW of shaft power for 0.05 kg/s at 2 bar Δp. - `EURange` values now come from the datasheet; `OverTempAlarm` reports the `BearingTemperature` chain (the correct `SourceNode` for an alarm whose limits are in Kelvin) rather than a boolean; `Cavitation` and `MotorOverheat` are derived from suction level and bearing temperature with hysteresis instead of `tick % n`. ### Diagrams The sample README gains a simulated-device summary, a link to the datasheet, and three Mermaid diagrams: address space, startup / hosting sequence, and simulation + alarm dataflow. ### Drift protection `PumpDatasheetConformanceTests` pins the nameplate, engineering ranges, trip points, published-value envelope and hydraulic consistency against the datasheet, and the exact pump node-surface baseline in `PumpInstanceNodeIdRegressionTests` covers the new identification properties. No changes to `src/` — this is sample, test and documentation only. ### Rebased on #4117 master has been merged into this branch. #4117 reshaped the same sample (loop-based pump creation via `--pumps N`, `Pump_1` / `Pump_2` BrowseNames with `Pump #n` DisplayNames, historian wiring, `WireBoolean`, per-pump `Diagnostics` group). Both intents are preserved: - the datasheet nameplate is applied through upstream's per-pump `WithIdentification(builder, pump, pumpNumber)` hook, so it scales to `--pumps N`; - `Program.cs` is left exactly as upstream has it — identification is configured centrally, so the old per-pump block there is gone; - the datasheet curve values are published through upstream's status-code / source-timestamp plumbing, and upstream's `WireBoolean` (TrueState/FalseState text + history) is kept; - the node-surface baseline is the union of both identification lists; - `docs/SourceGeneratedNodeManagers.md` was consolidated upstream into `docs/NodeManagers.md`, so the snippet fixes were ported there. ## Related Issues _No tracking issue; this is a documentation and sample-quality improvement to the pump reference sample._ ## 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 (post-merge) - `dotnet build samples/PumpDeviceIntegrationServer` (net10.0): 0 warnings, 0 errors - `dotnet build tests/Opc.Ua.Di.Tests` (net10.0): 0 warnings, 0 errors - `dotnet test tests/Opc.Ua.Di.Tests -f net10.0`: 331/331 passed (includes upstream's new `PumpAddressSpaceComplianceTests` and `DiConformanceFacetTests`) - `dotnet test tests/Opc.Ua.Di.Tests -f net48 --filter Pump`: 44/44 passed - Live client read against the merged sample: Δp 214 079 Pa, ṁ 8.654 kg/s, η 69.33 %, P 2 677.4 W — `Δp·(ṁ/ρ)/η` matches the published shaft power; `Pump_1` reads SN-001 / PMP-1001 / Feed Pump A / Bay 3 and `Pump_2` reads SN-002 / PMP-1002 / Feed Pump B / Bay 4, exactly as documented. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: fe608993-f3c9-4779-a6b9-9a9eabfc24ba	2026-08-04 10:37:36 +02: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
Marc SchierandCopilot	8a10139b8c	Fix OPC UA address-space compliance in the device integration stack (#4117 ) # Description A live OPC UA compliance audit of `samples/PumpDeviceIntegrationServer` against the OPC 40223 Pumps and OPC 10000-100 (DI) companion specifications found 13 address-space violations plus a serialization bug in the MCP tool. Every fix lands at its root cause in the shared stack rather than as a workaround in the sample, so all servers benefit. The sample now exposes N fully simulated pumps (`--pumps N`) and advertises the DI conformance facets it actually satisfies. > This branch has been merged with `master` including #4116 ("Add simulation to PumpDeviceIntegrationServer and Event modelling"), which independently reworked the same pump simulation and the same fluent alarm builders. See [Relationship to #4116](#relationship-to-4116) below for how the two were reconciled. ## Findings and fixes \| # \| Finding \| Root cause \| Fix \| \|---\|---------\|-----------\|-----\| \| 1 \| `OverTempAlarm` created by the fluent API did not exist in the address space \| `AttachAlarm` attached the alarm with `AddChild` but never indexed it into the node manager \| Register the node; the same gap existed in `AddObject`, `CreateInstance` and the state-machine creators \| \| 2 \| Alarms were unsubscribable — `EventNotifier = 0` everywhere, no `HasNotifier` from `Server` \| No event-source wiring at all \| Set `HasCondition`, initialise `SourceNode`/`SourceName`/`ConditionName`/`InputNode`, promote `EventNotifier` up the ancestor chain, publish `HasNotifier` from the `Server` object \| \| 3 \| Every instance NodeId was minted in the DI standard namespace (`ns=4;s=5001_Pump #1`) \| `DiNodeManager.New` inherited the parent's namespace index \| New `DiNodeManager.InstanceNamespaceIndex`; instances move to the server's application namespace \| \| 4 \| `Server/Namespaces` described only 2 of 6 namespaces \| The NodeSet2 importer drops nodes parented to namespace 0, so companion-spec metadata objects never reached the address space — and no route covered the server's own namespace \| New `NamespaceMetadataPublisher` walks the `NamespaceArray` and fills version/publication date from `ModelDependencyAttribute` \| \| 5 \| Vendor BrowseNames (`Diagnostics`, `LastError`, …) sat in the DI namespace \| Builders defaulted to the parent's namespace \| Default to the server namespace; spec-defined BrowseNames are unchanged \| \| 6 \| `AccessLevel="5"` from the Pumps NodeSet was reported as `1` \| The ModelDesign `AccessLevel` enum is not `[Flags]` and cannot represent `CurrentRead \\| HistoryRead` \| Carry the verbatim NodeSet2 bitmask and emit named `AccessLevels` constants \| \| 7 \| `Historizing = true` while `HistoryRead` returned `BadHistoryOperationUnsupported` \| Nothing reconciled the declared history surface with the wired historians \| Startup clears the advertisement (including masking the attribute read callbacks) when no provider resolves; the sample wires a real historian \| \| 8 \| Mandatory `ProductInstanceUri` unset \| Sample omission \| Populated on every pump \| \| 9 \| Mandatory `TrueState`/`FalseState` empty on every supervision boolean \| Absent in the official NodeSet; a server must still populate them \| Populated on every pump \| \| 10 \| Machinery `Machines` folder empty (OPC 40001-1 §9.2 says shall) \| `TryAddToMachinesFolder` used `AddChild`, which re-parents the device away from `DeviceSet` \| Add the `Organizes` reference instead \| \| 11 \| BrowseName contained a space and `#` \| Sample naming \| `Pump_1` with the readable label in `DisplayName` \| \| 12 \| `Maintenance` group materialised but empty \| Sample omission \| Populated \| \| 13 \| Uninitialised values reported `Good` \| No initial-value status \| `BadWaitingForInitialData` until the first simulation tick \| \| — \| `ServerProfileArray` advertised only `StandardUA2017` \| `DiNodeManager` did not implement the existing `IConformanceContributor` \| Contribute DI 1.05 server facets computed at runtime from what is actually wired, merged with `StandardUA2017` \| \| — \| MCP tool serialised boolean `false` and numeric `0` as JSON `null` \| `Variant.Null` is `default`, so `Variant.Equals` adopts the non-null type and compares against the default value \| Dispatch on `BuiltInType` and use typed `TryGetValue` accessors instead of the prohibited `AsBoxedObject` \| ## Relationship to #4116 #4116 landed on `master` while this branch was in flight and targeted overlapping ground. The merge is a union of both intents, not a fast-forward: - `AlarmBuilderExtensions` — kept both sides. #4116 contributes the parent-must-be-an-Object guard, `SetEnableState`, and the `HasEventSource` references; this branch contributes the `HasCondition` reference, condition source initialisation, registration of the alarm with the node manager so it is browsable, notifier promotion up the whole ancestor chain, and root-notifier registration. - `SupervisionBuilderExtensions` — took #4116's `SetAlarmActive`. It is more spec-correct than this branch's version: it honours `EnabledState`, resets Acked/Confirmed on activation, and computes `Retain` per OPC 10000-9 so an unacknowledged Condition stays retained after going inactive. This branch's null guard was kept so a malformed alarm cannot dereference a missing `EnabledState`. - `PumpNodeManager.Configure.cs` — took #4116's push-based simulation (`RegisterPumpSimulation`, `CreatePumpSimulation`, `IValueUpdater<T>`, `Initialize`/`Advance`/`Publish`, deterministic per-pump phase offsets) as the structure, then folded in this branch's compliance work: configurable pump count, Maintenance group, `TrueState`/`FalseState`, historian wiring, per-pump mandatory Identification values, and `BadWaitingForInitialData` until the first tick. - README — union, keeping #4116's address-space validation workflow section. Two test adjustments were needed, both because the merge legitimately changed behaviour the tests pinned: - `PumpHostedReferenceTests` now resolves pump NodeIds from the browse result instead of hard-coding the DI namespace, and waits for the first published value. `PumpCreatedAfterStartupJoinsTheLiveSimulationAsync` was added to restore #4116's coverage that a pump created after startup through `ConfigureDevicesFor` joins the live simulation. - `FluentAlarmRegistrationIntegrationTests` no longer asserts that `Retain` clears when an alarm goes inactive — that pinned this branch's weaker semantics, and #4116's Part 9 behaviour is correct. ## Breaking change Runtime-created DI device instance NodeIds move from the companion-spec namespace to the server's application namespace, e.g. `ns=<DI>;s=5001_Pump #1` becomes `ns=<application>;s=5001_Pump #1`. This invalidates NodeIds cached by clients from earlier 2.0 previews. It is not a model change — standard DI/Machinery/Pumps type NodeIds and spec-defined BrowseNames are untouched. Device integration hosting is new in 2.0, so there is nothing to migrate from 1.5.378 and this is deliberately not in `docs/MigrationGuide.md`. Anyone tracking 2.0 previews should rediscover devices by browsing `DeviceSet` rather than persisting NodeIds, and resolve namespace indexes from `NamespaceArray` per connection. ## Verification Compliance is pinned by regression tests that assert through the server's service surface (`BrowseAsync` / `ReadAsync` / `HistoryReadAsync`), not by inspecting `NodeState` objects in memory. That distinction mattered: three defects survived in-memory assertions and were only caught through the read path — most subtly, `Historizing` was cleared while `AccessLevel` still advertised `HistoryRead`, because attribute read callbacks re-applied the NodeSet bits. Post-merge on net10.0: \| Suite \| Result \| \|---\|---\| \| `Opc.Ua.Di.Tests` \| 326 passed \| \| `Opc.Ua.Server.Tests` \| 3846 passed, 5 skipped \| \| `Opc.Ua.Tools.Tests` \| 288 passed \| \| `Opc.Ua.History.Tests` \| 506 passed, 27 skipped \| \| `Opc.Ua.SourceGeneration.Core.Tests` \| 3722 passed, 8 skipped \| The full solution was also built and tested on net48 before the merge. Review feedback from `copilot-pull-request-reviewer` has been addressed in commit `badad31dc`: the shared `ServerObjectState` lock was removed (the forward `HasNotifier` edge is published through the owning node manager instead), MCP array serialization now preserves element types, and the pump option guards report the specific rejected option. The only failure is `ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync`, a pre-existing certificate-store issue that passes when run in isolation and is unrelated to this change. ## Documentation Updated `docs/DeviceIntegration.md`, `docs/MigrationGuide.md`, `docs/HistoricalAccess.md`, `docs/DeveloperGuide.md`, `docs/McpServer.md`, `docs/README.md`, `samples/PumpDeviceIntegrationServer/README.md` and `tools/Opc.Ua.Mcp/README.md`, and consolidated the four node-manager documents into a single [`docs/NodeManagers.md`](docs/NodeManagers.md) — overview, built-in managers (including `MasterNodeManager`), core vs custom, registration, server address-space metadata, and source generation — with a TOC and all inbound links repointed. ## Related Issues No tracking issue exists for this work yet — it originated from an ad-hoc compliance audit of the pump sample. Given the size of the change and the runtime-breaking NodeId move, please open (or link) a tracking issue so the design is recorded as an ADR before merging. - Fixes # ## 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. - [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. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 0d50c3fc-58b1-4c99-85a4-1e32c88666d9	2026-07-30 16:04:18 +02:00
Alexander KöpkeandCopilot	4251fec84d	Add simulation to PumpDeviceIntegrationServer and Event modelling (#4116 ) This pull request introduces significant improvements to the Pump Device Integration Server sample, focusing on robust event/alarm integration, simulation enhancements, and comprehensive test coverage. The changes ensure that pump objects and their events are correctly wired for OPC UA eventing, improve alarm state management and reporting, and update documentation and tests to validate these behaviors. Event and Alarm Integration Improvements: * Pump objects and their event sub-objects now have `EventNotifier` flags and are linked via `HasNotifier` references, ensuring they properly participate in the OPC UA event hierarchy. Each pump is also registered as a root notifier. [[1]](diffhunk://#diff-6d01bd352b39d1bd0a96e8c0c11d27c066ae33a64a552bdabe023535ca833fc2R297-R307) [[2]](diffhunk://#diff-6d01bd352b39d1bd0a96e8c0c11d27c066ae33a64a552bdabe023535ca833fc2R256-R257) * The alarm builder now enforces that alarms are attached only to valid object nodes, enables alarms by default, and wires `HasEventSource` references and `EventNotifier` flags for both parent and alarm nodes. [[1]](diffhunk://#diff-2d10b87cac23754d35da8632e9b1a23ec0c1e706689c7241da36dbc934b2666aR208-R215) [[2]](diffhunk://#diff-2d10b87cac23754d35da8632e9b1a23ec0c1e706689c7241da36dbc934b2666aR235-R248) Alarm State and Reporting Enhancements: * Alarm state transitions now check that the alarm is enabled before updating state, set acknowledged/confirmed flags, assign new event IDs, update timestamps, and report events using OPC UA mechanisms. * Alarm severity and message are set dynamically based on the active state, and the `Retain` flag is managed according to OPC UA alarm state requirements. Simulation and API Changes: * `CreatePumpAsync` now registers new pump instances with the simulation, ensuring all pumps participate in the live simulation loop. * Simulation ticks and monitored value publishing are unified and described in the documentation, with deterministic phase offsets for each pump. [[1]](diffhunk://#diff-e2a596af01b60e5bf0559e8c4307604ca80889523b67221fb999ce8a7ad2bb2cL98-R134) [[2]](diffhunk://#diff-e2a596af01b60e5bf0559e8c4307604ca80889523b67221fb999ce8a7ad2bb2cR178-R193) Documentation Updates: * The README is updated to reflect the new event/alarm integration, simulation details, and instructions for validating the address space with automated and manual workflows. [[1]](diffhunk://#diff-e2a596af01b60e5bf0559e8c4307604ca80889523b67221fb999ce8a7ad2bb2cL40-R73) [[2]](diffhunk://#diff-e2a596af01b60e5bf0559e8c4307604ca80889523b67221fb999ce8a7ad2bb2cL98-R134) [[3]](diffhunk://#diff-e2a596af01b60e5bf0559e8c4307604ca80889523b67221fb999ce8a7ad2bb2cR178-R193) Test Coverage Expansion: * The main integration test is expanded to verify that both pumps publish unique, changing monitored values and that alarm events are reported and observable via OPC UA subscriptions. [[1]](diffhunk://#diff-818e6fe65e5a28c2405bb287d9e40fcb74f1275c90755da56c60343528e17ee5L57-R58) [[2]](diffhunk://#diff-818e6fe65e5a28c2405bb287d9e40fcb74f1275c90755da56c60343528e17ee5R165-R314) These changes ensure the sample is a robust reference for OPC UA eventing, simulation, and alarm integration, and that its correctness is validated both automatically and through enhanced documentation.# Description _Describe the changes here to communicate to the maintainers why they should accept this pull request. By default - this will become the Commit message after merging and thus define history._ ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #github-issue-number, ... ## 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. - [ ] I have added all necessary documentation. - [ ] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed. - [ ] I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings. - [ ] I have addressed all PR feedback received. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-30 06:05:01 +02:00
Marc SchierandCopilot	cfd15f08e1	Assign instance NodeIds in generated CreateOrReplace<Child> helpers (#4104 ) # Description The model source generator emits two families of instance helpers with inconsistent NodeId behaviour: * `ISystemContext.CreateInstanceOf<Type>(parent, browseName)` rebases the whole materialised subtree onto per-instance NodeIds. * `<Owner>.CreateOrReplace<Child>(context, replacement)` — the plumbing behind `NodeState.CreateChild` / `ReplaceChild` — did not. The child it produced was either bare (null NodeId, no browse name) or kept the type-level NodeId of the replacement. Any node manager mixing the two had to hand-roll a recursive re-stamp before registration, or sibling instances collided on NodeIds — a failure that only surfaces at registration time. `samples/PumpDeviceIntegrationServer` did exactly this. ## What changed Generator (`tools/Opc.Ua.SourceGeneration.Core`) * `CreateOrReplace<Child>` gains `bool assignInstanceNodeIds = true`. When enabled and the context supplies an `ISystemContext.NodeIdFactory`, a child whose NodeId is null or still equal to its declaration NodeId — and its descendants — are rebased through `NodeInstanceExtensions.AssignInstanceNodeId` / `AssignInstanceChildNodeIds`, with the owner as reference root. A NodeId the caller already assigned is never overwritten. * The helper also stamps the new child's `SymbolicName` / `BrowseName` / `DisplayName`. Without an identity, a path-based `INodeIdFactory` derives `{parent}_` for every child of a parent, so the assignment would have introduced collisions rather than removed them. `BrowseName` stamping is guarded on `context.NamespaceUris != null` — event-reporting paths pass a context without a namespace table. * All generated `Create` factory call sites pass `assignInstanceNodeIds: false`: they build declaration subtrees whose NodeIds must stay at their type-level values, and the enclosing `CreateInstanceOf<Type>` factory rebases the finished subtree in a single pass. The `CreateInstanceOf<Type>` rebase guard drops the `parent != null` requirement — supplying a `browseName` is what marks a dynamically materialised instance. Calling it without one (as the generated `NodeStateActivator`s do, and when replacing a well-known singleton) still keeps the declaration NodeIds. * `replacement` is now emitted as `BaseInstanceState?` on `CreateOrReplace<Child>` and on the generated `FindChild` override, matching the documented contract and the base `NodeState.FindChild` signature. Hand-written twins — `MethodState.CreateOrReplace{Input,Output}Arguments` and `BaseDataVariableState.CreateOrReplaceEnumStrings` gain the same parameter and behaviour, sharing a new internal `NodeInstanceExtensions.AssignNewChildInstanceNodeIds`. This is also required so the generated `public new` overrides keep hiding a matching base signature (otherwise `CS0109` with `TreatWarningsAsErrors`). Sample — `PumpDeviceIntegrationServer` now passes the real `DeviceSet` parent to the factory and drops `AssignChildNodeIds` plus the manual root stamp. Docs — new "Materialising instances at runtime — NodeId assignment" section in `SourceGeneratedNodeManagers.md` and a refreshed `DeviceIntegration.md`. ## Design note: why not the guard proposed in the issue Gating on "the owner already has an instance NodeId" cannot be implemented as `owner.NodeId != <ownerTypeNodeId>`. The generated child factories call `<Owner>.CreateOrReplace<Child>` on declaration instances — `CreatePumpType_Operational` runs on a `FunctionalGroupState` whose NodeId is `ObjectIds.PumpType_Operational`, not `ObjectTypeIds.FunctionalGroupType` — so the comparison would be true while the type template is being materialised (`forInstance: false`) and would rewrite the address space's declaration NodeIds. The explicit opt-out separates the two callers unambiguously. ## Compatibility Adding an optional parameter is source-compatible. Behaviour changes for callers of `NodeState.CreateChild` / `ReplaceChild` when the context carries a `NodeIdFactory`: the new child now carries a browse name and a per-instance NodeId instead of nothing. ## Related Issues - Fixes #4099 > [!NOTE] > The issue asks to delete `AssignChildNodeIds` from `samples/MinimalRobotServer`. That sample does not exist on `master` — it only lives on the still-open #4096 branch and carries no such helper. The equivalent boilerplate on `master` is `PumpDeviceIntegrationServer/PumpNodeManager`, which this PR cleans up. Details in [this comment](https://github.com/OPCFoundation/UA-.NETStandard/issues/4099#issuecomment-5102275507). > [!NOTE] > One commit is an unrelated one-line-scope build-hygiene fix: two `CA2000` warnings in `ConsoleReferenceClient` where subscription ownership transfers to the session. Happy to split it out if preferred. ## 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. - [x] I have addressed all PR feedback received. ### Tests added Generator emission (`NodeManagerGeneratorTests`) — `CreateOrReplaceChildAssignsInstanceNodeIdsByDefault`, `TypeFactoriesOptOutOfCreateOrReplaceNodeIdAssignment`, `CreateInstanceOfFactoriesRebaseWithoutAnExplicitParent`. Helper behaviour (`NodeInstanceExtensionsTests`) — `CreateOrReplaceArgumentsAssignsInstanceNodeIds`, `CreateOrReplaceArgumentsKeepsCallerAssignedNodeId`, `CreateOrReplaceArgumentsHonoursTheAssignmentOptOut`, `CreateOrReplaceEnumStringsAssignsInstanceNodeIds`. Pump DI server regression (`PumpInstanceNodeIdRegressionTests`, added on review feedback): \| Test \| Asserts \| \| --- \| --- \| \| `ConfiguredPumpExposesTheFullModelSurface` \| The exact set of 77 model identifiers a configured pump materialises. \| \| `EveryPumpInstanceExposesTheSameGeneratedModelSurface` \| A second pump exposes the identical generated surface, minus what the fluent `Configure` pass adds. \| \| `MaterialisedPumpNodeIdsComeFromTheNodeIdFactory` \| Every NodeId is `{parentIdentifier}_{symbolicName}` in the parent's namespace — minted by `PumpNodeManager.New`. \| \| `MaterialisedPumpNodeIdsAreUniqueAcrossInstances` \| No NodeId is null or shared between two instances of the type. \| \| `GeneratedFactoryAssignsInstanceNodeIdsBeforeRegistration` \| The factory output already carries per-instance NodeIds before `AddPredefinedNodeAsync` applies its defensive rebase. \| \| `AlarmSubtreeKeepsStandardDeclarationNodeIds` \| Pins the alarm subtree's standard-namespace ids — a pre-existing gap outside the generated helpers. \| The 77-identifier baseline was captured by running the same subtree walk against the pre-change commit and diffing it against the post-change run: identical — same node set, same NodeIds. ### Local test runs \| Suite \| net10.0 \| net48 \| net472 \| \| --- \| --- \| --- \| --- \| \| `Opc.Ua.SourceGeneration.Core.Tests` \| 3722 passed \| 3721 passed \| — \| \| `Opc.Ua.SourceGeneration.Tests` \| 81 passed \| 81 passed \| — \| \| `Opc.Ua.SourceGeneration.Stack.Tests` \| 90 passed \| — \| — \| \| `Opc.Ua.Types.Tests` \| 8083 passed \| 8076 passed \| — \| \| `Opc.Ua.Di.Tests` \| 303 passed \| 303 passed \| 277 passed \| \| `Opc.Ua.Core.Tests` \| 4134 passed \| — \| — \| \| `Opc.Ua.Gds.Tests` \| 1095 passed \| — \| — \| \| `Opc.Ua.History.Tests` \| 506 passed \| — \| — \| \| `Opc.Ua.Server.Tests` \| 3626 passed, 1 failed \| 3574 passed, 1 failed \| — \| The single `Opc.Ua.Server.Tests` failure — `ServerFluentApiHostingTests.ConfigureApplicationBuildsSharedClientAndServerConfigurationAsync` — is pre-existing and unrelated; it reproduces on a stashed clean `master` tree. > [!NOTE] > The source-generation test harness compiles generated model code against a hand-written `Opc.Ua` API stub (`tests/Opc.Ua.SourceGeneration.Core.Tests/CompilerUtils.cs`, shared by three test projects). Its 73 `CreateOrReplace<Child>` declarations were updated to the new signature — without that, every generated call site passing `assignInstanceNodeIds: false` failed to bind. `NodesetMethodArgumentTests` pinned the exact emitted call and was updated to the new one for the same reason as the `NodeStateGeneratorTests` expectations. > [!NOTE] > `master` has been merged into this branch. The only conflict was in `NodeStateGeneratorTests.cs`, where master rewrapped the expected call-site strings while this branch appended `assignInstanceNodeIds: false` to them — kept master's wrapping and re-applied the argument. Master's two new templates (`Create_MethodArguments`, `Assign_MethodArgumentValues`) call `CreateOrReplace{Input,Output}Arguments` while building a declaration subtree, so they now pass `assignInstanceNodeIds: false` to preserve their stated intent of leaving the property node's identity untouched. > [!TIP] > Changing a source generator leaves dependent projects' cached generated output stale. If a local build reports `CS1739: ... does not have a parameter named 'assignInstanceNodeIds'` or `CS0109` on a generated `CreateOrReplace*`, run `dotnet build <project> -t:Rebuild`. Clean CI builds are unaffected. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: b019f3a2-f58d-4f7e-93db-c8a0dec656e2	2026-07-28 21:22:14 +02:00
Marc Schier	e5cb64cb62	Make fluent sample security settings explicit (#4087 ) # Description Makes security-critical configuration explicit in the modern fluent client and server samples instead of relying on hidden defaults. - Adds an application-wide `ConfigureSecurity` DI callback for advanced certificate validation settings. - Makes PKI roots and certificate validation settings explicit in the Pump, Boiler, Calculator, and Minimal Client samples. - Makes the Calculator sample's endpoint policies and anonymous authentication choice explicit. - Adds consolidated developer guidance for application certificates, endpoint security, identity, resource controls, and every advanced security-builder setting. - Preserves the samples' existing automatic-trust behavior while clearly marking it as sample-only. ## Related Issues - Fixes #4085 ## 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. - [ ] I have addressed all PR feedback received. Copilot-Session: 833a8855-af3c-4a85-887b-32bf38d92cfa	2026-07-24 12:56:51 +02:00
Alexander Köpke Marc Schier Marc Schier Copilot	6a92fbd1a1	Link EuRange correctly from PumpDeviceIntegrationServer (#4075 ) This pull request improves the registration of property nodes created via the fluent API, ensuring that properties such as `EngineeringUnits` and `EURange` are always properly registered with the node manager. It introduces a utility class to centralize node registration and updates both the implementation and tests to reflect this behavior. Fluent API improvements: * Refactored `EnsureEngineeringUnitsProperty` and `EnsureEURangeProperty` to always register created properties using the new `FluentNodeRegistration.RegisterCreatedNode` method, ensuring consistent node manager registration. [[1]](diffhunk://#diff-4128c152ec415df1746b49a2e1df0e18cb05e5381c3e5884c6724a0e2c2f1b61L92-R92) [[2]](diffhunk://#diff-4128c152ec415df1746b49a2e1df0e18cb05e5381c3e5884c6724a0e2c2f1b61L132-R132) [[3]](diffhunk://#diff-4128c152ec415df1746b49a2e1df0e18cb05e5381c3e5884c6724a0e2c2f1b61L169-R191) * Updated property creation in `PropertyInitBuilderExtensions.cs` to use the centralized registration utility. New utility for node registration: * Added the `FluentNodeRegistration` class to encapsulate logic for registering newly created nodes with an `AsyncCustomNodeManager`. Testing improvements: * Added new tests to verify that `EngineeringUnits` and `EURange` properties are both browsable and readable, and that they are properly registered with the node manager. [[1]](diffhunk://#diff-84c2ea5e8e977638cf52c62a81a90c62f67b2d78efd70db62b3ef39c2f86adc4R136-R198) [[2]](diffhunk://#diff-84c2ea5e8e977638cf52c62a81a90c62f67b2d78efd70db62b3ef39c2f86adc4R281-R331) [[3]](diffhunk://#diff-8ff5510d21a0f088a52b9f7be699cc08a4033e9514d2fb491bdfaa55e2d50b27R88-R157) * Refactored and renamed test files and classes for clarity, updating test harnesses and node manager mocks to support the new registration logic. [[1]](diffhunk://#diff-8ff5510d21a0f088a52b9f7be699cc08a4033e9514d2fb491bdfaa55e2d50b27L32-R33) [[2]](diffhunk://#diff-8ff5510d21a0f088a52b9f7be699cc08a4033e9514d2fb491bdfaa55e2d50b27L42-R50) [[3]](diffhunk://#diff-8ff5510d21a0f088a52b9f7be699cc08a4033e9514d2fb491bdfaa55e2d50b27L118-R187) [[4]](diffhunk://#diff-8ff5510d21a0f088a52b9f7be699cc08a4033e9514d2fb491bdfaa55e2d50b27R197-R248) [[5]](diffhunk://#diff-8ff5510d21a0f088a52b9f7be699cc08a4033e9514d2fb491bdfaa55e2d50b27R263-R268)# Description _Describe the changes here to communicate to the maintainers why they should accept this pull request. By default - this will become the Commit message after merging and thus define history._ ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #github-issue-number, ... ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [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. - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@hotmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-23 05:31:47 +02:00
Alexander Köpke Marc Schier Marc Schier Copilot	0de25de89a	Change PumpDeviceIntegrationServer to use PumpType (#4066 ) This pull request updates the Device Integration documentation and the `PumpDeviceIntegrationServer` sample to introduce and demonstrate the new `ITopologyElementBuilder<TElement>` API for working with OPC UA Device Integration topology elements. The changes clarify the distinction between device and topology-element builders, update sample code to use the new builder, and improve the overall clarity and accuracy of both the documentation and the sample implementation. API and Documentation Updates: * Introduced `ITopologyElementBuilder<TElement>` as the unified builder for topology elements, exposing common operations such as functional groups and topology references, and updated all references and examples to use this builder where appropriate. [[1]](diffhunk://#diff-93e580c17580d8cb3c5c4206fdf6b64c1deabc1e00e4f83d64583661a97f76eaR164-R173) [[2]](diffhunk://#diff-93e580c17580d8cb3c5c4206fdf6b64c1deabc1e00e4f83d64583661a97f76eaR219-R228) [[3]](diffhunk://#diff-93e580c17580d8cb3c5c4206fdf6b64c1deabc1e00e4f83d64583661a97f76eaL897-R926) [[4]](diffhunk://#diff-93e580c17580d8cb3c5c4206fdf6b64c1deabc1e00e4f83d64583661a97f76eaL912-R943) [[5]](diffhunk://#diff-93e580c17580d8cb3c5c4206fdf6b64c1deabc1e00e4f83d64583661a97f76eaL926-R958) * Clarified the distinction between device builders (`IDeviceBuilder<TDevice>`) and topology-element builders in both the documentation and sample code, and documented the available builder methods and their intended use. [[1]](diffhunk://#diff-93e580c17580d8cb3c5c4206fdf6b64c1deabc1e00e4f83d64583661a97f76eaL33-R38) [[2]](diffhunk://#diff-93e580c17580d8cb3c5c4206fdf6b64c1deabc1e00e4f83d64583661a97f76eaL141-R146) [[3]](diffhunk://#diff-93e580c17580d8cb3c5c4206fdf6b64c1deabc1e00e4f83d64583661a97f76eaR495-R500) Sample Code Modernization: * Refactored `samples/PumpDeviceIntegrationServer/Program.cs` to use the new `ITopologyElementBuilder<TElement>` API for creating and configuring the second pump, including setting identification properties and functional groups, and removed legacy code for device health and software update facets. [[1]](diffhunk://#diff-7e065631de4a55cb923ba4fa9a32fffcff804ce5ef3e09a70a2eead76425b18dL74-R98) [[2]](diffhunk://#diff-7e065631de4a55cb923ba4fa9a32fffcff804ce5ef3e09a70a2eead76425b18dL133-L139) [[3]](diffhunk://#diff-7e065631de4a55cb923ba4fa9a32fffcff804ce5ef3e09a70a2eead76425b18dL53-L57) [[4]](diffhunk://#diff-7e065631de4a55cb923ba4fa9a32fffcff804ce5ef3e09a70a2eead76425b18dL37-R37) * Cleaned up and modernized the sample project description and removed unused or outdated fields and comments. [[1]](diffhunk://#diff-e3976eed0bf7d7b96d6762d58e8101a0670b2ef60aa04609e8d5ff4be30f64beL8-R8) [[2]](diffhunk://#diff-70693dd3c39308e8e25d4e009281950f860a914afcf92b2e100a0710b5bb8db1L58-L76) [[3]](diffhunk://#diff-70693dd3c39308e8e25d4e009281950f860a914afcf92b2e100a0710b5bb8db1L110-R91) [[4]](diffhunk://#diff-70693dd3c39308e8e25d4e009281950f860a914afcf92b2e100a0710b5bb8db1L129-R110) Software Update Example Improvements: * Updated the software update walkthrough in the documentation to use the new builder APIs and improved the clarity of the example, including more accurate references to the simulated device and software update facet. [[1]](diffhunk://#diff-5342d39e79e59ca4fc77a9d6277377841c321d17476979f612c0487c53342238L300-R303) [[2]](diffhunk://#diff-5342d39e79e59ca4fc77a9d6277377841c321d17476979f612c0487c53342238L313-R337) [[3]](diffhunk://#diff-5342d39e79e59ca4fc77a9d6277377841c321d17476979f612c0487c53342238L378-L379) These changes make it easier to work with OPC UA Device Integration topology elements, provide clearer guidance in the documentation, and ensure the sample code reflects current best practices.# Description _Describe the changes here to communicate to the maintainers why they should accept this pull request. By default - this will become the Commit message after merging and thus define history._ ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #github-issue-number, ... ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [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. - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@hotmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-22 04:06:03 +02:00
Alexander Köpke Marc Schier Marc Schier Copilot	a568a7e54c	Set ContinuationPoint (when browse is finished) to `null` instead of empty array, align with existing 1.5.xxx behavior (#4057 ) This pull request standardizes the handling of the `ContinuationPoint` property in `BrowseResult` objects, ensuring it is always explicitly set to `null` (or its equivalent) when no continuation point is present. This change improves clarity and consistency in both the implementation and the tests, replacing checks for an "empty" continuation point with checks for a "null" continuation point. Codebase consistency and correctness: * All `BrowseResult` objects now explicitly set `ContinuationPoint = default` when no continuation point is to be returned, ensuring consistent initialization. (`src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs`) [[1]](diffhunk://#diff-851e5925c677925c42bd70bce11ea1a75a5f5cbdb810ac851351385e16f54a71L1900-R1904) [[2]](diffhunk://#diff-851e5925c677925c42bd70bce11ea1a75a5f5cbdb810ac851351385e16f54a71L2068-R2076) [[3]](diffhunk://#diff-851e5925c677925c42bd70bce11ea1a75a5f5cbdb810ac851351385e16f54a71L2078-R2090) Test updates for new semantics: * All test assertions now use `ContinuationPoint.IsNull` instead of `IsEmpty` to check for the absence of a continuation point, reflecting the new contract. (`tests/Opc.Ua.InformationModel.Tests/BrowseContinuationPointTests.cs`, `tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs`) [[1]](diffhunk://#diff-904275d6b4bee35e6884f38e0db5cbc955216eaf2aa7f468ba285133470e1596L121-R122) [[2]](diffhunk://#diff-904275d6b4bee35e6884f38e0db5cbc955216eaf2aa7f468ba285133470e1596L394-R394) [[3]](diffhunk://#diff-904275d6b4bee35e6884f38e0db5cbc955216eaf2aa7f468ba285133470e1596L435-R435) [[4]](diffhunk://#diff-904275d6b4bee35e6884f38e0db5cbc955216eaf2aa7f468ba285133470e1596L634-R634) [[5]](diffhunk://#diff-bda1da5dc01533ffe61d1c65b6e968b6462e11a63758ac95ded000ffc885345eL420-R420) * Additional test cases have been added to verify that completed browse operations and error scenarios always return a null continuation point. (`tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs`) [[1]](diffhunk://#diff-3d9efc81935ec60d7513334833e9eab033925194f5a0d11105668204cc60f54aR203-R230) [[2]](diffhunk://#diff-3d9efc81935ec60d7513334833e9eab033925194f5a0d11105668204cc60f54aR327) [[3]](diffhunk://#diff-3d9efc81935ec60d7513334833e9eab033925194f5a0d11105668204cc60f54aR349)# Description _Describe the changes here to communicate to the maintainers why they should accept this pull request. By default - this will become the Commit message after merging and thus define history._ ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #github-issue-number, ... ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [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. - [ ] 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: Marc Schier <marcschier@users.noreply.github.com> Co-authored-by: Marc Schier <marcschier@hotmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-07-21 20:54:19 +02:00
Marc Schier	437a0dadc7	Merge origin/master into marcschier/3907-repository-layout	2026-07-16 07:11:36 +02:00
Marc Schier	8538f531e7	Update paths for the consolidated repository layout	2026-07-16 06:26:05 +02:00
Marc Schier	7cec2d614d	Move repository files to the consolidated layout	2026-07-16 06:25:54 +02:00