Skip to content

Robot intent: a task-level command API with a Part 10 lifecycle - #4165

Open
marcschier wants to merge 21 commits into
OPCFoundation:masterfrom
marcschier:marcschier/robot-intent
Open

Robot intent: a task-level command API with a Part 10 lifecycle#4165
marcschier wants to merge 21 commits into
OPCFoundation:masterfrom
marcschier:marcschier/robot-intent

Conversation

@marcschier

@marcschier marcschier commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

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 codeOpc.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.

ServerIntentControllerHost 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.

ClientRobotIntentClient 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 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 AddRobotIntentConfigureRobotIntent 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:

  • Coordinate the robot cell, and make the twin show what it is actually doing #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.
  • OpenUSD scene: replace object in the USD value model with a UsdValue union #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 unvalidatedPick.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).
  • 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

The companion specification is drafted in the open at marcschier/opcua-drafts#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

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

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 Robotics: the OPC 40010 companion SDK, its client and the robot sample #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.

marcschier and others added 3 commits August 2, 2026 14:35
The ten motion verbs merged in OPCFoundation#4127 established a vocabulary and left the
harder half undone. They were synchronous: no operation handle, no progress,
no server-side cancel, no queueing, no ownership - and the file said as much,
describing itself as a non-normative convention.

That shape cannot work. A motion takes seconds and a pick takes a minute,
while OPC 10000-4 discards a method result when the Session ends "independent
of the task actually performed at the Server". A synchronous motion method
therefore loses the outcome of work that has already physically happened.

This replaces the convention API with an implementation of the OPC UA - Robot
Intent draft (metaverse-specs/robot-intent in marcschier/opcua-drafts).
Submission returns a Part 10 program instance the client watches, which is
the resolution OPC 10000-10 already reaches for exactly this case.

The model is source-generated from its NodeSet, so the enums, the polymorphic
intent structures, IntentOperationState : ProgramStateMachineState and the
typed clients are all derived from the specification rather than hand-copied
from it. Verbs are a DataType hierarchy, so a submission and a mission step
are the same shape and a new intent is a subtype rather than a new method -
which is what AddOperation<TRequest,TResponse> was working around.

IntentControllerHost owns admission in the specification's order, the queue
with PLCopen buffer modes, cancellation with the server's right to refuse,
missions with an immutable committed base and a revisable horizon, and the
capability declaration. 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.

Three things the tests found, each of which would have been a defect in the
field rather than a test artifact:

  - The result was published after the state went terminal, so a client
    acting on the transition read a null result. It is now published first.
  - Capabilities were resolved against whatever namespace table existed when
    they were declared, so the list silently matched nothing. They now
    resolve when the host starts, and are published to the address space so
    the declaration a client reads is the one the host enforces.
  - FinalResultData and the optional folders are Optional in their type
    definitions and so were never materialised. A server that implements a
    facet has to expose its optional members or the facet is unclaimable.

124 tests pass, 24 of them new: the admission order, every state pairing in
the specification's table, buffered ordering, supersession reported as
Superseded rather than as a cancellation, a refused cancel, an accepted one,
retry as a new attempt that leaves the original's history intact, and the
mission base refusing to be altered.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1dab1302-19c5-4a9b-a50c-d97d389713aa
Follows the specification's enlarged scope: safety awareness, trajectories
and force, brokered real-time channels, and the mission step graph.

Safety is a report plus a refusal duty. UpdateSafetyState is how the
application tells the host what the safety system is enforcing; admission
then refuses on the same values a client can read, so a refusal is
explainable from the address space rather than from state only the Server can
see. Ready reflects it too - a client told Ready and then refused has been
told something untrue. Only an explicit Cartesian speed is compared against
the safe limit: a speed FRACTION is of a configured maximum the host does not
know, and refusing what cannot be judged would reject legitimate work.

Trajectories are validated wholly at admission, because a trajectory is
handed over in one call and there is no later exchange in which to complain:
ascending time, per-point axis count, and the declared point limit.

Channels are described and leased, never carried. While a lease is held the
host refuses motion intents unless it declares that it arbitrates, because
two things commanding one robot with no arbitration is the failure that rule
exists to prevent.

The mission engine gained the step graph and the five error policies.
Compensate differs from Fallback only in what happens after the fallback step
succeeds, and that is where the distinction is implemented.

Two bugs the tests found, both of which would have been silent in the field:

  - A lease taken by a caller with no Session left the holder null, so the
    channel still looked free and a second caller could take it. The lease is
    now tracked explicitly rather than inferred from the holder.
  - An empty ContentFilter arrives as an empty element array rather than a
    null filter, so testing only for null made every unconditional transition
    silently untaken - a mission would run its first step and stop. Both null
    and empty now mean unconditional.

149 tests pass, 25 of them new: the safety refusals and the limit that is not
being enforced, trajectory ordering and bounds, force parameter validation,
lease exclusivity and mode gating, motion refused beside a held lease and
admitted when the host arbitrates, each error policy, an unconditional
transition choosing the next step, transitions ignored when branching is not
declared, and a mission without transitions still being the flat sequence.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1dab1302-19c5-4a9b-a50c-d97d389713aa
The intent code used five APIs that do not exist on net472/net48, which the
library targets: a default interface implementation, ArgumentNullException.
ThrowIfNull, string.Create with an interpolated handler, ValueTask.FromResult
and CompletedTask, and the generic Enum.GetValues.

None of them were load-bearing. The null checks now match what the rest of
this library already does, the interpolation uses FormattableString.Invariant,
and the ValueTask results use the struct constructor.

IIntentExecutor.CanCancel loses its default implementation and becomes a
required member. That is not only a portability fix: whether a motion can be
safely abandoned part-way is a decision worth making deliberately rather than
inheriting, and an executor with no such motions writes one line to say so.

The one place a language-version conditional is warranted is the test that
enumerates ExecutionStateEnum, because enumerating rather than listing is the
point - a state added without a clause 6.3 pairing has to fail there.

149 tests pass on net48 and on net10.0, with no warnings on either.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 1dab1302-19c5-4a9b-a50c-d97d389713aa
@marcschier

Copy link
Copy Markdown
Collaborator Author

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.

No problem

Should the model live in Opc.Ua.Robotics at all? 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. It sits here because this is where the robot-> facing API already is.

Keep in Opc.Ua.Robotics

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.

No, fully remove now

Serial execution is a deliberate simplification. Concurrent None/Soft intents would be an optimisation, not a correction, but it > is worth agreeing that reading.

Yes, but we need both

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR replaces the earlier opt-in “Robotics operation conventions” verb surface with a task-level Robot Intent API modeled as Part 10 program instances: clients submit an intent/mission, receive a handle (NodeId) back, and observe execution/progress/results asynchronously. It also wires Robot Intent into the Robotics package via source-generation from a NodeSet and updates client/server/test code accordingly.

Changes:

  • Add Robot Intent contracts, host options, and the IntentControllerHost execution engine (admission, queueing, cancellation, missions, real-time channel leasing, safety refusal).
  • Add extensive NUnit coverage for lifecycle rules, missions, safety gating, trajectories/force validation, and channel leasing.
  • Remove the previous non-normative operation convention builder/client API and update Robotics client accessors and builder interfaces.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tests/Opc.Ua.Robotics.Tests/RoboticsOperationsConventionBuilderTests.cs Removes tests for the old non-normative convention methods API.
tests/Opc.Ua.Robotics.Tests/IntentScopeExtensionTests.cs New tests covering extended intent scope (safety, trajectories/force, channels, missions).
tests/Opc.Ua.Robotics.Tests/IntentControllerHostTests.cs New tests validating the intent execution lifecycle, queueing, cancellation, retry, and mission base/horizon rules.
src/Opc.Ua.Robotics/RoboticsOperationConventions.cs Removes the old convention request/result types and enums.
src/Opc.Ua.Robotics/Opc.Ua.Robotics.csproj Adds the Robot Intent NodeSet as an AdditionalFiles input for source generation.
src/Opc.Ua.Robotics/Intent/IntentContracts.cs Introduces executor/progress contracts and outcome structures for intent execution.
src/Opc.Ua.Robotics.Server/Intent/IntentControllerHostOptions.cs Adds capability/channel/safety/mission validation and configuration structures.
src/Opc.Ua.Robotics.Server/Intent/IntentControllerHost.cs Adds the Robot Intent host implementation (methods, pump, node materialization, leasing, missions).
src/Opc.Ua.Robotics.Server/Builders/RoboticsOperationsBuilders.cs Removes the old convention operations builder implementation.
src/Opc.Ua.Robotics.Server/Builders/MotionBuilders.cs Removes AddOperations(...) hook for the old convention API.
src/Opc.Ua.Robotics.Server/Builders/MotionBuilderInterfaces.cs Removes AddOperations(...) from the motion builder interface.
src/Opc.Ua.Robotics.Client/RoboticsClient.Accessors.cs Replaces convention operations accessor with an IntentController(...) client entry point.
src/Opc.Ua.Robotics.Client/Operations/RoboticsOperationsClient.cs Removes the old convention operations client implementation.
Suppressed comments (2)

src/Opc.Ua.Robotics.Server/Intent/IntentControllerHost.cs:99

  • The constructor API/docs still expose a removeNode parameter, but the corresponding field is unused (and should be removed to keep the build warning-free). After removing the field, the constructor signature/body should also drop the parameter and assignment to avoid compile errors and reduce misleading API surface.
        /// <param name="removeNode">Removes a node again, when the host can delete.</param>
        public IntentControllerHost(
            IntentControllerState controller,
            IIntentExecutor executor,
            Func<NodeState, CancellationToken, ValueTask> addNode,

src/Opc.Ua.Robotics.Server/Intent/IntentControllerHost.cs:598

  • ExceedsSafeSpeed reads m_safety without synchronization, which can race with UpdateSafetyState and produce inconsistent decisions. Since this method is called outside of lock (m_lock), it should snapshot safety state via the locked SafetyState accessor (or otherwise synchronize).
        private bool ExceedsSafeSpeed(IntentDataType intent)
        {
            if (!m_safety.SafeSpeedLimitActive || m_safety.SafeSpeedLimit <= 0)
            {
                return false;

Comment thread src/Opc.Ua.Robotics.Server/Intent/IntentControllerHost.cs Outdated
Comment thread src/Opc.Ua.Robotics.Server/Intent/IntentControllerHost.cs Outdated
Comment thread src/Opc.Ua.Robotics.Server/Intent/IntentControllerHost.cs Outdated
Comment thread src/Opc.Ua.Robotics/Intent/IntentContracts.cs Outdated
marcschier and others added 5 commits August 4, 2026 11:03
Sync root is System.Threading.Lock, not object, per the repo guideline and
the polyfill in Opc.Ua.Types. Locking on an object also lets unrelated
code lock the same instance by accident.

m_removeNode was assigned and never read - an unused private field, which
warnings-as-errors rejects. It is wired now rather than deleted, because
the delegate was covering a real leak: an operation instance survives the
work it describes so a client can read the result afterwards, and nothing
then removed it, so a controller that runs continuously accumulated an
operation node for every intent it had ever been given.
RetainedTerminalOperations bounds that, and defaults to zero - keep
everything - so behaviour is unchanged unless a host opts in. Only
terminal operations are pruned; one still queued or executing is going to
change again and a client watching it would lose the rest of the story.

Admission read m_safety without synchronisation and called
AnyChannelHeldLocked() without holding the lock its name claims. Every
check ran unsynchronised and the mutation then happened under a separate
acquisition, so a stop asserted in that window admitted work the Server
had already been told to refuse. Admission is now decided and acted on
under one acquisition.

The same time-of-check/time-of-use shape appeared in eight other places -
cancel, pause, resume, the real-time lease, mission submission and
mission update all checked authority under the lock, released it, then
mutated under a second acquisition. All folded in. HoldsAuthority is gone
because nothing calls it any more, and leaving it would have reintroduced
the unused-member problem above. ExceedsSafeSpeed and FindCapability are
renamed *Locked to state the contract they now rely on.

Shape validation stays after the authority check rather than moving ahead
of the lock: AuthorityIsCheckedBeforeParameters guards that ordering, and
it is right to - a caller holding no authority should not learn from the
answer whether its parameters would have been valid.

IntentContracts.cs is split one type per file: IIntentProgress,
IntentExecution, IntentOutcome, IIntentExecutor.

149/149 robotics tests pass.
OPC 40010 describes a robot in detail and defines no motion verbs at all: its
whole 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. This implements the draft OPC UA - Robot Intent companion model,
which supplies the verbs and nothing else, so the two compose rather than
compete.

The shape of the model follows from one constraint. An OPC UA Call cannot stay
open for the length of a real motion, and OPC 10000-4 discards a method result
when the Session ends "independent of the task actually performed at the
Server" - so a synchronous method that commands a robot loses the outcome of
work that has already physically happened. SubmitIntent therefore returns as
soon as the intent is admitted, and what it returns is a Part 10 program
instance the client subscribes to for progress and reads for the result.

Model and shared code
  Opc.Ua.Robotics source-generates the Robot Intent NodeSet, whose only
  RequiredModel is the base UA namespace, so a server can adopt it without
  pulling in 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 keeps a pole orientation from 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.

Server
  IntentControllerHost admits, queues, executes and reports, with all thirteen
  Methods wired, command authority released on Session close, real-time channel
  leases that lapse, clause 11.3 validation of every client-supplied NodeId
  against the controller being commanded, constraint clamping, trajectory
  tolerances, blending that completes the predecessor when blending begins, and
  Part 10 transition events. 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; and fluent builders cover every intent and mission.

Samples
  All robotics samples move under samples/Robotics. MinimalIntentRobotServer is
  one UR5e-style offset-wrist arm on a bench, declared completely enough to show
  what a conformant server publishes, including 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.

Specification defects found by implementing it were fixed upstream in
marcschier/opcua-drafts PR 47 rather than worked around here: refusal was
unobservable because SubmitIntent returned no failure, the Part 10
ProgramDiagnostic promotion declared a second member instead of promoting the
inherited one, WaitIntentDataType.Signal was unbounded where clause 11.3 has to
check it, and clause 9's honesty rule said nothing about the Method surface.

Changed-line coverage is 89.9 percent with no file below 80. The suite is 388 on
net10.0 and 370 on net8.0, net9.0, net472 and net48; the live client-to-server
integration suite is 11. A clean rebuild of UA.slnx emits no new warning.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
A test-quality audit of the previous commit found the suite protects the
hard parts -- clause 11.3 scoping, base immutability, the safety refusal
set, the asin clamp, refusal-as-Good -- but named three holes the coverage
figure concealed, and five tests whose names asserted a normative
requirement while their code asserted something weaker or nothing at all.

The worst was a test that fabricated the answer it checked.
ReadControllerSafeAsync 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 at all.
The decode failure no longer happens, so the fallback was hiding nothing
and is gone; the suite now passes with honest reads, three times over.

Chasing that led somewhere larger. The facet calculator did not compute
RI-Blending, RI-Queue or RI-Mission-Horizon at all, granted RI-Grasp
without checking the ToolType had a TcpFrame, and RI-Description on
nothing more than Description != null -- and the tests asserted the weak
behaviour, so they locked it in. Worse, the client re-derived facets
independently and disagreed with the server on essentially every row:
its 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.

That is a specification defect, and it is fixed upstream rather than
worked around here (opcua-drafts PR 47, 3d92474). Clause 12 defined
conformance in terms of facets and gave a Server nowhere to publish which
ones it had, so every client had to re-derive Table 12.2 from the address
space -- and since several rows are behavioural, no amount of browsing
settles them. IntentCapabilitiesType now carries SupportedFacets, exactly
as OPC 10000-5 does with ServerCapabilitiesType.ServerProfileArray, and
clause 12.2 separates structural requirements a Server shall meet before
listing a facet from attested ones governed by clause 9. The server
publishes what the calculator computes; the client reads it.

The publication test earned its place immediately by failing on the
wiring added alongside it: MaxQueueDepth lived only in host options, not
on the state node the calculator reads, so the server under-claimed
RI-Queue while actually queueing.

Also fixed, each proven by a named regression test:

  - entry.Execution was published outside the lock, so CancelIntent could
    fault on CanCancel(null) and the requested StopMode was silently lost.
  - Dispose enumerated m_intents while mutating it.
  - A shutdown timeout abandoned the pump and skipped DisposeResources;
    cleanup is now deferred to pump completion, and the node manager
    leaves the address space standing rather than tearing it down under
    an executor that may still touch it.
  - Retention ordered by tie-prone StartTime; it now uses admission
    sequence.
  - Viewport picking was completely dead: CreatePickRequest ran inside
    Task.Run, so Avalonia's VerifyAccess threw off the UI thread, and
    because the renderer reported as started the CommandPrim fallback was
    suppressed too. The request is now built before the thread hop.

Annex C had no independent oracle for roll or yaw -- transposing them
left the whole suite green while destroying the interop property the
conversion exists for. There are now hand-derived oracles for each axis
and a composite rotation, in both directions; deliberately swapping A and
C turns them red. Forward kinematics likewise had no oracle, because
Inverse calls Forward for its own error metric and a systematic error
cancels.

Robotics tests 388 -> 445 (net10.0) and 370 -> 426 (net48); integration
11 -> 20, now gate-driven rather than wall-clock and faster for it.
Changed-line coverage 90.66%, no file below 80%.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
Three things, all following from the same principle: a conformance claim
a Server publishes has to be true, and it should not be possible for it
to become false by accident.

RI-Interop-40010 was the last facet the calculator did not compute, and
the reason it documented for that was wrong. It claimed the facet could
not be inferred "because the calculator receives only a Robot Intent
controller, not the linked OPC 40010 model". But Annex B's structural
requirement is a HasIntentController reference *from* the
MotionDeviceSystemType *to* the controller -- an inverse reference on the
controller, reachable from IntentControllerState without any handle on
the OPC 40010 model. The integration suite was already browsing exactly
that relationship. So a Server that genuinely implemented Annex B could
not claim Annex B. It can now.

SupportedFacets was an eager snapshot taken during RegisterAsync, so a
reference attached afterwards left the published claim wrong. Patching
the known builder path was the wrong shape of fix: it left the invariant
depending on every future caller remembering to refresh. The variable is
now bound to the calculator through BindRead and recomputed on every
read, so the claim tracks the address space by construction rather than
by discipline. Attaching HasIntentController after registration now works
as well as before it, which matters for anyone bolting Robot Intent onto
an existing OPC 40010 node manager -- they often cannot control that
ordering. The node is marked read-only: a client writing the Server's own
conformance claim would be absurd.

The test that was supposed to pin that read-only marking could not fail.
It asserted AccessLevel equals CurrentRead, but BaseVariableState
initialises every variable to exactly that, so deleting MarkReadOnly left
it green. It is replaced by a write expecting BadNotWritable, and by a
real OPC UA Write service call over a live session, which is the only
thing that proves a client cannot actually do it.

Also fixes 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
run so far.

Verification, the first time this change set has been tested beyond four
projects: full UA.slnx rebuild 0 errors; 50 of 52 test assemblies green
on net10.0; Robotics 449 (net10.0), 430 (net48) and 426 on each of
net472, net8.0 and net9.0 -- none of which had ever been run before;
integration 20; AOT 121/121.

The three failures in the full run are not from this branch. Two fail
identically at the merge-base ae241b6: a culture-sensitive assertion
expecting "12.5" against a machine producing "12,5", and a
certificate-rotation teardown count. The third, AOT FindServersAsync with
BadSecureChannelClosed, does not reproduce and passed 4/4 on retest.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
Four conflicts, all from the two branches touching the same robot and
OpenUSD surfaces.

master added six files to samples/MinimalRobotServer -- CellChoreographer,
CellLayout, RobotAgent, RobotArmSolver, RobotCell.Twin and
RobotKinematics, from the robot cell choreography work -- while this
branch moved that directory to samples/Robotics/MinimalRobotServer. The
new files are kept at the moved location, so both intents survive.
UA.slnx takes the union: the three WotCon sample projects master adds,
plus the three Robotics projects, with master's MinimalRobotServer entry
rewritten to its new path.

RobotAssetContractTests conflicted because master replaced object in the
USD value model with a UsdValue union (OPCFoundation#4160) and rewrote Flatten to
match, while this branch had converted the same method to a block body
and corrected the sample asset path in its doc comment. The merge left a
hybrid that did not compile. The resolution carries both: master's
UsdValue arm and its Flatten(UsdValue) overload, in this branch's block
body, with the corrected path retained.

OpenUsdConnectorRunner was the only conflict that needed care. master
inserted FindStageCamera immediately before WriteStageUsda, and because
this branch had relocated WriteStageUsda and GetPrivateStateRoot during
the member-order pass, the merge kept master's insertion together with
its trailing context and produced a second copy of both methods. Only
FindStageCamera is genuinely new -- master's diff shows the other two as
context, unmodified -- so the duplicates are dropped and this branch's
internal versions kept, which are the same code plus the
GetPrivateStateRoot(string?) overload the connector tests use to avoid
writing into the real user profile. master's --camera documentation is
taken over this branch's, being a superset.

One conflict surfaced only at build time: master moved the whole OpenUSD
stack from 0.1.0-alpha to 0.4.0-alpha, and this branch pins
OpenUsd.Rendering in the test project with a VersionOverride so the
renderer-pick tests can see the picking backend. Left at 0.1.0-alpha that
pin is lower than the version the viewer now resolves transitively, which
is CS1705. The pin follows master to 0.4.0-alpha.

Verified: dotnet build UA.slnx 0 errors 0 warnings; Opc.Ua.OpenUsd.Tests
813 (master contributes ~149 of those), Opc.Ua.Robotics.Tests 449 on
net10.0 and 430 on net48, Opc.Ua.RobotIntent.Integration 20.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
Comment thread docs/DeveloperGuide.md Outdated
Comment thread docs/MigrationGuide.md Outdated
Comment thread docs/RobotIntent.md Outdated
Comment thread docs/README.md Outdated
Comment thread docs/WhatsNewIn2.0.md Outdated
Comment thread samples/Robotics/IntentEnabledRobot/IntentEnabledRobot.csproj
marcschier and others added 6 commits August 6, 2026 20:56
Review comments, in order.

Integration tests belong with their unit tests. The DeveloperGuide now
says so: a feature library's integration tests live in <Component>.Tests,
every test project name ends in .Tests, and they are split out only when
they run long, destabilise the unit tests, or the suite needs dividing.
Opc.Ua.RobotIntent.Integration was wrong on both counts. Measuring it
decided the rest: it is NonParallelizable, stands a real server up on a
dynamic port, adds around 55 seconds, and -- something not previously
checked -- runs on net48 as well. Merging it into Opc.Ua.Robotics.Tests
would have put live-server tests into a fast unit assembly on all five
frameworks, which is the impact the rule exists to prevent, so it is
renamed Opc.Ua.Robotics.Intent.Tests instead.

MigrationGuide covers 1.5.378 only, and the Robot Intent section
described an unreleased-2.0 prototype removal, so it is gone.
RobotIntent.md is merged into Robotics.md as a section rather than a
sibling document, folding the duplicated Packages and See also sections
and keeping the draft-status warning near the top. The README and
WhatsNewIn2.0 entries are folded down to the sentence each warranted.
MinimalIntentRobotServer is renamed IntentEnabledRobot.

The rest came from reviewing the change properly, five reviewers over
disjoint areas.

A mission whose next step was refused reported Succeeded, because the
advance path returned the same false for "steps exhausted" and "step
refused". 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 and ToolChange.DockStation -- because each switch
arm returned a single check and fell out. 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.

CanCancel and the mission condition evaluator ran under the host lock
while the executor's own thread re-entered it through ReportBlendBegin. A
fieldbus-consulting CanCancel therefore deadlocked CancelIntent and
CancelAll: a lost stop. A non-terminal executor outcome threw from
outside the try/catch that exists because the executor is untrusted,
killing the pump while Ready stayed true and submissions kept being
accepted. Aborting overwrote an already-terminal result, discarding a
blend's AchievedPose. A complete terminal Result was published when a
cancel was merely accepted, so a supervisor polling Result saw the
operation ended while the arm was still decelerating.

Pause published Suspended while nothing told the executor to pause, so
the robot kept moving under an HMI reporting position retained. Pause is
now queue-only and honest about it, and the specification gained the
matching rule (opcua-drafts 357a558): a Server that cannot suspend a
running intent declares PauseSupported false.

RI-RealTimeChannel was granted on an empty folder with neither Open nor
Close method, and RI-Safety on the mere presence of a Mandatory node --
vacuous by construction, and my own earlier misreading of the modelling
rule ids had put it there. It now requires a bound safety source. Retry
was created unconditionally, so a controller browsed a method its
SupportedFacets omitted. The node manager's deferred-shutdown path was
unreachable in-server and leaked when it did run.

CommandAuthorityLease dropped Granted on its own first notification --
monitored items report the current value, which is the owner just
granted -- so authority appeared lost the instant it was acquired. An
operation handle could complete with an empty Result because the
transport discarded node identity and the pump guessed by CLR type. A
Null Variant in an output turned a refusal into a thrown fault, which
clause 6.2 forbids and which any third-party server leaving outputs
unset would have triggered.

Renderer picking never worked. The reflective probe bound
OpenUsdStormRenderer, which is not a picking backend but happens to
expose a matching Pick, and whose implementation throws off its owner
thread. The probe now binds only IRenderPickingBackend, failures fall
back immediately and loudly, and the documentation says plainly that
renderer picking is unavailable with this package and why.

The Part 10 promotions never promoted: namespace-1 BrowseNames declared
second members beside the inherited ones rather than overriding them, and
the conformance test could not see it because its lookup stripped the
prefix. Both fixed, the test now asserts the raw name.

Circular and force moves advanced the tool pose while passing unchanged
joint angles, so the twin froze while the server reported an arc. They
solve IK per step now and fail when it does not converge -- which turned
out to be a joint limit, not the singularity I assumed. The reference
sample also advertised CallProgram it did not implement and reported
SetOutput success without touching an output.

Verified: UA.slnx rebuild 0 errors and the 2 pre-existing CA1861
warnings; Opc.Ua.Robotics.Tests 467 on net10.0 and 447 on net48;
Opc.Ua.Robotics.Intent.Tests 20 on both; Opc.Ua.OpenUsd.Tests 818;
Opc.Ua.Aot.Tests 121/121.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
IntentEnabledRobot pins net10.0 for the OpenUSD payload, so it cannot
take part in a UA.slnx build pinned to a legacy TFM through
CustomTestTarget. On those 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:

  error CS0433: The type 'Lock' exists in both 'Opc.Ua.Types' and
  'System.Runtime'

which failed seven CI jobs across net472, net48, netstandard2.0,
netstandard2.1 and net8.0 while net9.0 and net10.0 passed.

The repository already has the opt-out for exactly this: setting
RestrictForLegacyTfm makes the project a no-op build on legs it cannot
join. Opc.Ua.OpenUsd.Connector.Viewer, which is net10.0-pinned for the
same reason, already uses it; this sample simply never opted in.

Verified locally with CustomTestTarget set to each of net472, net48,
netstandard2.0, netstandard2.1 and net8.0 - all build clean - and the
default multi-TFM build still succeeds with only the two pre-existing
CA1861 warnings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
The Linux CI leg failed CircularMoveKeepsPublishedToolPoseConsistentWith
Joints where Windows passed. Chasing it found a real defect rather than a
platform quirk.

SetPose already computed Forward(m_jointAngles) and published
forward.JointFramePoses from it, but published the *commanded*
interpolated pose as the tool pose. The two differ by the
inverse-kinematics residual, so the tool centre point and the rendered
joint frames were derived from different sources and disagreed - about
8 mm here, and by a constant amount after any real motion. That is the
same lying-digital-twin defect the per-step IK change set out to remove,
one level further in: solving IK per step stopped the joints going stale,
but the pose published beside them was still the one that had been asked
for rather than the one reached. SetPose now takes only the joint angles
and derives both from them, so the invariant holds by construction.

The test earned its keep twice. It was also probing the edge of the
workspace: from the home configuration the interpolated arc grazes a
joint limit, which resolved as a failure on Windows and a success on
Linux, so its outcome assertion was pinning a platform-dependent
accident. It now pre-positions to a configuration well inside the limits
and asserts success, matching what SimulatedArmExecutorTests already
does; the unsolvable-path behaviour stays pinned deterministically by
ForceIntentFailsWhenInterpolatedPathCannotBeSolved.

Mutation-checked: publishing the commanded pose again, or passing stale
joints at the circular call site, both turn the test red.

Verified: OpenUSD 818, Robotics 467 net10.0 / 447 net48, Robot Intent 20.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
The clause 6.2 fix - a Null Variant in an output position must not turn a
refusal into a thrown fault - was implemented in ObjectTypeProxyGenerator
rather than in one client, so every generated proxy gets it. That changed
the emitted shape: the local is now declared first, a Null output takes
the type's default, and only a non-Null output goes through TryGetValue.

Three generator tests asserted the old single-expression form and failed
on the net48 leg. They are pre-existing tests that this PR's behaviour
change legitimately invalidates, so they are updated rather than the
behaviour reverted - and strengthened while updating: they now also pin
the IsNull branch and the default assignment, so the interop fix itself
is covered rather than merely tolerated.

Opc.Ua.SourceGeneration.Core.Tests: 3771 passed, 0 failed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
ForceIntentContactSucceeds failed on the CI net10.0 leg inside DrainAsync
while passing locally in about 0.6 s. The executor runs on a manual clock
so these tests do not depend on machine speed, but the drain loop guarded
itself with a five second wall-clock deadline, which put the machine
dependence straight back. CI runs the suite with coverage instrumentation
enabled, and that alone is enough to take a per-step inverse-kinematics
probe from well under a second to over five.

The guard is now a simulated step budget, which is deterministic on any
machine and under any instrumentation. The elapsed deadline is kept only
as a generous backstop so a genuine hang still fails rather than hanging
the run. No assertion under test changed.

Verified the way CI runs it, Release with coverage collection and the
repository runsettings: 467 passed, 0 failed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
One conflict, in docs/DeveloperGuide.md, where both sides deliberately
rewrote the coverage-gate section.

master (OPCFoundation#4158) replaced it wholesale: patch coverage is now graduated by
patch size through patch.bands, with the smaller bands warning rather
than failing, plus new sections on Codecov being informational-only and
on where the numbers surface in each CI system. All of that is kept.

This branch had added a paragraph about the ignore globs, prompted by
losing a round to them: samples/** is ignored, but tools/** is not, so
anything under tools/ is measured like product code -- and an assembly no
test project references contributes changed lines counted as uncovered,
because no report mentions them. I checked that against the merged
coverage-thresholds.json rather than assuming: the ignore list still
carries tests/**, samples/**, obj, bin and *.g.cs, with tools/** absent,
so the warning still applies and is kept, sited ahead of master's new
subsections.

master's "reproduce a coverage failure locally" line supersedes this
branch's near-identical one, so only master's survives.

Verified: UA.slnx rebuild 0 errors and the 2 pre-existing CA1861
warnings; Opc.Ua.Robotics.Tests 467 on net10.0 and 447 on net48;
Opc.Ua.Robotics.Intent.Tests 20 on both.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
@marcschier
marcschier marked this pull request as ready for review August 7, 2026 06:41
@marcschier
marcschier requested a review from Copilot August 7, 2026 06:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

marcschier and others added 6 commits August 7, 2026 09:15
Every nonce-derived ECC key operation failed on macOS:

  System.PlatformNotSupportedException: Only named curves are supported
  on this platform.
    at ECDsaSecurityTransforms.ExportExplicitParameters(Boolean)
    at AdditionalEntropyCertificateKeyGenerator.CreateEcdsaKey

Apple's SecurityTransforms ECDsa implementation supports named curves
only, so ExportExplicitParameters throws there while working on Windows
and Linux. CreateEcdsaKey called it for one value: the order n of the
group generated by the base point, needed to reduce a DRBG draw into a
private scalar in [1, n-1].

OPC 10000-12 admits exactly four named curves here - nistP256, nistP384,
brainpoolP256r1 and brainpoolP384r1, per
CryptoUtils.GetCurveFromCertificateTypeId - and their orders are
published constants, so they are carried directly. Any other curve still
goes through the platform export, and a platform that cannot do it now
reports Bad_NotSupported naming the curve instead of surfacing a raw
PlatformNotSupportedException.

Carrying cryptographic constants by hand is worth doing only if they
cannot silently drift, so CurveOrderTableMatchesThePlatform checks each
one against the platform's own exported order wherever explicit export
works - every Windows and Linux leg - and ignores itself where it does
not. A mistyped digit would produce scalars outside the group order, so
this is the assertion that makes the approach safe rather than merely
convenient. All four match on Windows.

This is a pre-existing defect on master, not something this branch
introduced; it surfaced because the CI split in OPCFoundation#4158 added macOS legs
that had never run before. It accounts for the two ECC failures in
test-macOS-latest-Server. The other macOS failures on master (Gds,
Tools, Redundancy.Samples, aot-macos) have different causes and are not
addressed here.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
Convert.ToHexString is .NET 5+, so the new CurveOrderTableMatchesThePlatform
cases broke the net48 and net472 legs of Opc.Ua.Server.Tests - my own
regression, on the target frameworks I had not built before pushing.

Replaced with a StringBuilder hex rendering that compiles everywhere. The
assertion is unchanged; all four curves still match the platform's exported
order on net10.0 and net48.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
test-ubuntu-latest-Sessions failed intermittently in global teardown:

  Certificate leak detected: 2 instance(s) created but not disposed
  (created=46905, disposed=46903)

with every test passing. The detector polled Certificate.InstancesLeaked
for a fixed five seconds and then reported whatever it saw, which cannot
separate the two cases it exists to tell apart. A genuine leak holds the
count steady; a suite still draining its fire-and-forget channel and
session disposals has a count that is falling. On a loaded agent that
drain outlasts any budget short enough to keep a real leak's failure
prompt - so two of nearly forty-seven thousand certificates were still in
flight when the assertion ran.

It now waits for quiescence: a decrease resets the clock, and the count
is reported only once it has stopped moving for several consecutive
reads. That makes a real leak surface sooner than the old fixed budget
did, while a slow drain gets as long as it keeps making progress. The
poll stays hard-bounded and still never calls WaitForPendingFinalizers,
so it cannot hang the test host.

This is shared across every suite that asserts no certificate leaks, so
it removes the whole flake class rather than one instance of it. The
assertion itself is unchanged - a count that settles above zero still
fails.

Verified: Opc.Ua.Sessions.Tests 777 passed, no teardown failure.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
One conflict, in tools/Opc.Ua.OpenUsd.Connector/OpenUsdConnectorRunner.cs,
across four hunks. Both sides had extended the same file, so all four are
resolved as a union rather than a choice.

master (OPCFoundation#4180) adds site-level server composition: OpenRemoteSessionAsync
opens a session to a server named by a cross-server component binding,
and FetchFederatedAssetsAsync pulls the asset closure of every such
server into the shared cache directory. Both are kept whole, along with
the OpenUsdConnectorOptions parameter they thread through
RunViewportAsync.

This branch had extended the same method signature for the viewport pick
work - printPickCommands, commandPrimPath and PickMode. The call site and
the signature now carry both sets of parameters.

The remaining two hunks are the same trap this file sprang during the
previous merge. master's additions arrive with their trailing context,
which re-declares WriteStageUsda and GetPrivateStateRoot - members this
branch relocated during the member-order pass and made internal, adding
the GetPrivateStateRoot(string?) overload the connector tests use to stay
out of the real user profile. Taking that context verbatim would compile
to duplicate members, so only the genuinely new methods are kept. The
orphaned "Writes a self-contained stage.usda" comment that travels with
them is dropped again for the same reason: it belongs to WriteStageUsda,
which already carries it where it now lives.

Verified: UA.slnx rebuild 0 errors and the 2 pre-existing CA1861
warnings; Opc.Ua.OpenUsd.Tests 904 (818 before, the rest arriving with

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
OPCFoundation#4180); Opc.Ua.Robotics.Tests 467; Opc.Ua.Robotics.Intent.Tests 20.
OpenUSD 0.5.0-alpha closes all 11 issues filed against
marcschier/openusd-dotnet, so the workarounds built around them go.

- Delete the viewer reflection probe entirely (-827 lines). Renderer
  picking now uses ViewerHostOptions.PrimPicked, so --pick-mode Renderer
  produces picks for the first time and Auto no longer degrades on sight.
  ViewerStageSession.PickingBackend/Camera/CurrentRenderState are public,
  so no private-field walking or Application.Current spelunking remains.
- Author primvars:displayColor through SetColor3fArray. The pump casing
  colour and the generator thermal colour published correctly but had
  never moved on screen; they animate now.
- Use the RID-agnostic OpenUsd.Runtime.* metapackages. The win-x64-only
  claim was never true - linux-x64 and osx-arm64 shipped from 0.1.0-alpha
  - so the viewer widens to net8.0;net9.0;net10.0 and the false platform
  statements are corrected. Adds a non-blocking Linux SwiftShader render
  smoke job.
- Drop the three "known viewport limitations" (colour, materials,
  cameras); all three are fixed upstream.

Spec compliance, from a four-reviewer pass against the merged draft:

- PauseSupported now defaults false. The host only stops queue dispatch
  and IIntentExecutor has no suspend member, so declaring true broke
  clause 9 rule 4 and told operators a moving robot had paused.
- Safety state reaches the admission gate. WithSafetyState bound only the
  published nodes while the gate read a field nothing wrote, leaving
  every clause 10.4 refusal dead; the gate now refreshes at admission, so
  it no longer depends on a client happening to read SafetyState.
- The non-DI path no longer admits work it cannot perform. Adds
  WithExecutor, passes the service provider through, and fails loudly at
  build time instead of substituting a rejecting executor.
- SubmitMission follows the clause 6.2 refusal order, reports the
  specific IntentFailureEnum instead of ParameterInvalid for everything,
  applies the clause 10.4 gates, and never creates an operation instance
  for a refusal.
- Aborting supersedes unconditionally per clause 6.4; Parallel mission
  divergence is refused rather than silently dropped; an unresolved
  branch fails the mission instead of reporting Succeeded; cancel results
  are reported honestly and forward StopMode.
- Annex B operational modes must agree, and RI-Interop-40010 is withheld
  when they do not. Publishes the clause 12.4 profile and facet URIs.

Also fixes 8 latent nullable defects in IntentEnabledRobot and repairs
the only link to the specification, through which 41 clause citations
resolve.

Spec defects found during this pass are raised upstream in
marcschier/opcua-drafts#60.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
The job failed on CI hardware: a linux-x64 publish of
OpenUsd.Rendering.Silk.Vulkan with OpenUsd.Runtime.Imaging does not
contain libopenusd_hdsilk.so, so OpenUsdSilkRuntime.Create throws
DllNotFoundException before anything is rendered.

It was already non-blocking, but a check that is permanently red on
every run is noise that teaches people to ignore CI, and it exercised
the upstream packages rather than this repository. Filed upstream as
marcschier/openusd-dotnet#12; restore the job once that is resolved.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
The pump's BearingTempColor binding moved from the shader input
/Body/Mat/Surface.inputs:diffuseColor to /Body.primvars:displayColor,
which is the canonical per-prim colour and the target the connector
README documents. displayColor could not be authored before
openusd-dotnet#2 was fixed, which is why the binding pointed at the
material in the first place.

The E2E assertion still expected the superseded target.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a248a589-9a20-4372-868e-5d347e57001b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants