Robot intent: a task-level command API with a Part 10 lifecycle - #4165
Draft
marcschier wants to merge 3 commits into
Draft
Robot intent: a task-level command API with a Part 10 lifecycle#4165marcschier wants to merge 3 commits into
marcschier wants to merge 3 commits into
Conversation
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
8 tasks
Collaborator
Author
No problem
Keep in Opc.Ua.Robotics
No, fully remove now
Yes, but we need both |
Contributor
There was a problem hiding this comment.
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
IntentControllerHostexecution 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
removeNodeparameter, 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
ExceedsSafeSpeedreadsm_safetywithout synchronization, which can race withUpdateSafetyStateand produce inconsistent decisions. Since this method is called outside oflock (m_lock), it should snapshot safety state via the lockedSafetyStateaccessor (or otherwise synchronize).
private bool ExceedsSafeSpeed(IntentDataType intent)
{
if (!m_safety.SafeSpeedLimitActive || m_safety.SafeSpeedLimit <= 0)
{
return false;
| private const uint StateSuspended = 3; | ||
| private const uint StateHalted = 4; | ||
|
|
||
| private readonly object m_lock = new(); |
Comment on lines
+68
to
+70
| private readonly Func<NodeState, CancellationToken, ValueTask> m_addNode; | ||
| private readonly Func<NodeState, CancellationToken, ValueTask>? m_removeNode; | ||
| private readonly Dictionary<string, IntentEntry> m_intents = []; |
Comment on lines
+188
to
+212
| if (!m_safety.PermitsSubmission) | ||
| { | ||
| return IntentAdmission.Refused(IntentFailureEnum.NotPermittedInMode, | ||
| m_safety.SafetyControllerOk | ||
| ? "A stop is asserted." | ||
| : "The safety controller reports a fault."); | ||
| } | ||
| if (intent == null) | ||
| { | ||
| return IntentAdmission.Refused(IntentFailureEnum.ParameterInvalid, | ||
| "No intent was supplied."); | ||
| } | ||
| if (intent is MotionIntentDataType && AnyChannelHeldLocked() && | ||
| !m_options.ArbitratesWithRealTimeChannel) | ||
| { | ||
| return IntentAdmission.Refused(IntentFailureEnum.CapabilityNotSupported, | ||
| "A real-time channel lease is held and this Server does not " | ||
| + "arbitrate between the two command sources."); | ||
| } | ||
| if (ExceedsSafeSpeed(intent)) | ||
| { | ||
| return IntentAdmission.Refused(IntentFailureEnum.SafetyLimitExceeded, | ||
| FormattableString.Invariant( | ||
| $"The requested speed exceeds the enforced safe limit of {m_safety.SafeSpeedLimit} m/s.")); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Update the task-level robot intent API added earlier: submit a motion or a task, get a handle back, watch it to completion. 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,Stopand loading a named program. #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
SubmitIntentreturns as soon as the intent is admitted, and what it returns is a NodeId — anIntentOperationState(a Part 10 program instance) the client subscribes to for progress and reads for the result.What changed
Opc.Ua.RobotIntent.NodeSet2.xmlis added toOpc.Ua.Roboticsas anAdditionalFilesentry, so the enums, the polymorphic intent structures,IntentOperationState : ProgramStateMachineState, the method states and the typed clients all derive from the model rather than being hand-written.AddOperation<TRequest,TResponse>was working around.IntentControllerHostowns admission (in the specification's order), the queue with PLCopen buffer modes, 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.BlockingModeconstraint by construction: the specification forbids beginning aSingleorHardintent while another executes and merely permitsNoneandSoftto overlap.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
RequestSPDUholds 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:
UpdateSafetyStateis 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.Bugs the tests caught
Each of these would have been silent in the field:
FinalResultDataand the optional folders areOptionalin 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.ContentFilterarrives 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.Related Issues
There is no issue for this yet; opening as a draft to agree the design first. Happy to open a tracking issue and record the decision there if that is preferred.
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
On the test checkbox: the robotics suite — 149 tests, 25 of them new — passes on net48 and net10.0, with zero warnings on both, and the sample and client projects build. I have not yet run the whole
UA.slnxacross all five target frameworks, so the box stays unticked until I have.Worth noting that requirement earned its keep immediately: it caught five APIs that do not exist on .NET Framework (a default interface implementation,
ArgumentNullException.ThrowIfNull,string.Createwith an interpolated handler,ValueTask.FromResult/CompletedTask, and genericEnum.GetValues), none of which the net10.0 build had complained about.Points I would most like feedback on
Opc.Ua.Roboticsat all? It is standalone on the base UA namespace and takes no dependency on OPC 40010, so a separateOpc.Ua.RobotIntentpackage would arguably be cleaner. It sits here because this is where the robot-facing API already is.None/Softintents would be an optimisation, not a correction, but it is worth agreeing that reading.