Coordinate the robot cell, and make the twin show what it is actually doing - #4162
Open
marcschier wants to merge 13 commits into
Open
Coordinate the robot cell, and make the twin show what it is actually doing#4162marcschier wants to merge 13 commits into
marcschier wants to merge 13 commits into
Conversation
The cell now has two transfer stations at opposite ends (WorkTableA and WorkTableB) with painted slots, a Parts scope whose members carry a live matrix transform, charging docks and painted travel lanes, replacing the single central work table whose parts were static children that nothing ever picked. Adds RobotKinematics, which composes the tool centre point from the link offsets authored in robot.usda and the same six axis values the server publishes, so a carried part cannot drift away from the gripper. Verified against a hand computed home pose. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
The sample ran the robots on independent figure-eight paths whose platforms cleared by 1.2 m while the arms reach 1.61 m, so the arms swept through each other even though the README claimed the footprints cleared. Replaces the open loop paths with a coordinated transfer cycle. The cell is divided into two end zones, a corridor with one lane per direction of travel and a southern layby holding the charging docks. A robot reserves the zone ahead before entering it and releases the one behind, and an arm may only leave its transport envelope inside an exclusively reserved end zone - which is what stops the arms meeting. A robot with no work parks on its dock so the station it serves stays free for the other robot to deliver into, without which the cell deadlocks on the first handover. R1 now carries parts from the western station to the eastern one and R2 carries them back, with travel on a trapezoidal accel/cruise/decel profile, proximity slowdown, battery drain and charging, injected grip faults with recovery, and cycle KPIs. The invariants are asserted directly over a ten minute simulated run: oriented footprints never overlap, a zone is never doubly occupied, arms only deploy in an end zone, parts are conserved and a carried part tracks the tool centre point exactly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
The simulation tick now advances the choreographer and publishes the axis positions it produces, replacing the canned eight pose loop that was unrelated to where the robot or the table actually was. Both robots carry a gripper, and the emergency stop halts the cycle rather than only blinking the beacon. The dynamic tool demo and the speed override command target were keyed on "has a tool", which quietly moved to R2 once both robots had one; they are keyed on R1 explicitly now. RobotPositionsDriveLiveOpenUsdSceneAsync polled a fixed 1.2 s window and assumed the robot was always moving. Under a duty cycle a robot legitimately stands still while gripping, dwelling or charging, so it now waits until movement is observed - the assertion is that live positions reach the scene, not that the robot never stops. README documents the zoning, the traffic rules, the transfer cycle and the further realism ideas, and corrects the claim that the platform footprints made the two robots clear each other. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Part03 now starts on the eastern station instead of the western one. Previously every part began on table A, so R2 stood idle for the ~24 s it took R1 to complete its first traverse before any work existed for it - which also made the end to end position test depend on that dead time. The handover test is strengthened accordingly: it now tracks a part that *started* on the western station reaching the eastern one, rather than merely observing a part on table B, which staging one there would otherwise satisfy trivially. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the MinimalRobotServer sample to run a coordinated two-robot transfer cycle (with zone reservations, arm stow/deploy rules, part pick/place, and richer simulation state) to prevent the manipulators from colliding while still allowing both robots to operate concurrently and pass in the corridor.
Changes:
- Adds a shared
CellChoreographer-driven duty cycle (zones/lanes/docks, e-stop halting motion, charging/fault injection, KPIs) and publishes its poses/axes through the existing server model. - Introduces forward/inverse kinematics (
RobotKinematics,RobotArmSolver) and uses FK to keep carried parts rigidly aligned with the published axis values. - Adds long-run invariant/regression tests (kinematics and choreography), updates E2E polling to account for legitimate robot idle periods, and updates the USD cell asset + README to match the new behavior.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Opc.Ua.Di.Tests/RobotOpenUsdE2eTests.cs | Adjusts the OpenUSD live-position E2E to poll for motion under the new duty cycle. |
| tests/Opc.Ua.Di.Tests/RobotKinematicsTests.cs | Adds unit tests validating forward kinematics and the arm solver against FK. |
| tests/Opc.Ua.Di.Tests/Opc.Ua.Di.Tests.csproj | Skips the new robotics tests on net472 (where the MinimalRobotServer reference is excluded). |
| tests/Opc.Ua.Di.Tests/CellChoreographyTests.cs | Adds long-run invariant tests to prevent collisions, double-occupancy, and part/pose inconsistencies. |
| samples/MinimalRobotServer/RobotKinematics.cs | Implements rigid transforms + FK used to compute the tool center point from published axis values. |
| samples/MinimalRobotServer/RobotCell.cs | Gives R2 a tool, fixes the “R1 keyed by tool” coupling, and plumbs choreographer identity into axis runtime. |
| samples/MinimalRobotServer/RobotCell.Configure.cs | Drives published axis values from the choreographer; e-stop now halts choreography. |
| samples/MinimalRobotServer/RobotArmSolver.cs | Adds IK for placing the TCP onto target slot coordinates. |
| samples/MinimalRobotServer/RobotAgent.cs | Introduces the per-robot simulation state model (pose, axes, battery, zone state, etc.). |
| samples/MinimalRobotServer/README.md | Documents the new zoning/traffic rules, transfer cycle, and corrects the prior clearance claim. |
| samples/MinimalRobotServer/Program.cs | Registers CellChoreographer in DI for the hosted sample. |
| samples/MinimalRobotServer/MobileRobotPositionProvider.cs | Uses the shared choreographer for robot poses when hosted; retains options-driven motion for standalone tests. |
| samples/MinimalRobotServer/CellLayout.cs | Defines authoritative cell geometry constants (stations, lanes, docks, slot positions). |
| samples/MinimalRobotServer/CellChoreographer.cs | Implements zone reservation + coordinated motion + part handling + faults/charging/KPIs. |
| samples/MinimalRobotServer/Assets/Cell.usda | Updates the USD scene to two stations, lanes, docks, and live-authored circulating parts. |
The robots drove the cycle and the arms reached the slots, but nothing was picked up or moved: the choreographer tracked part poses and gripper opening purely in memory. The address space only ever exposed the platform poses and the twelve axis angles, so a connector had nothing to subscribe to for the parts and the blocks stayed wherever the asset authored them. Add a CellTwin folder holding a ThreeDCartesianCoordinates position and a ThreeDOrientation heading per part, plus a jaw position per robot, and bind them to the part prims and to the gripper jaws. The variables are registered before the motion device system is built, because a binding can only carry a SourceNodeId that already exists, and they sit outside the robotics subtree: a workpiece is not a device, and the robotics builder validates every node below its root as one it allocated itself. The poses are published as structured vectors rather than as a matrix or a bare angle. A viewer composes a prim's matrix from the vector translate and rotate ops; the matrix4d profile is explicitly left unresolved by the reference connector, which drops a binding it cannot convert in silence. That is the shape the first attempt took, and it failed without a word. The jaws needed the asset restructured. A carrier cube and a finger mesh were siblings under the gripper body, so driving the carrier would have slid it out from under the finger it is bolted to. Each jaw is now one Xform owning both, declaring the single matrix op every live-bound prim declares, with the finger points rebased onto the jaw origin. Closed puts the finger faces on the 35 mm half-width of the block, so the gripper grips it rather than passing through. Extend RobotAssetContractTests to hold every live-bound prim to the single-matrix contract, not just the two robot mount points. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Collecting the lowest free slot meant a part that never landed in slot 0 was never collected at all. With two parts seeded on WorkTableA the second sat untouched for the life of the process while the robots shuttled the other one past it, which reads in the twin as the robots ignoring a block sitting in front of them. Stamp each part with the order in which it came to rest and collect the one that has waited longest, which is what a real cell does with a buffer anyway. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
A part clearly floated outside the jaws in the viewport. The platform pose and the workpiece pose leave the server as separate bindings on separate loops: the positioning provider published at 5 Hz and the simulation tick at 20 Hz, so the part - which rides the tool centre point - was drawn against a platform up to four ticks stale. Measured against the live scene with the arm stowed, where the offset is fixed by construction, the part sat anywhere between 0.65 m and 0.90 m from its platform instead of at one distance: a quarter of a metre of drift, three part widths, in front of a 96 mm jaw opening. Publish the platform at the tick rate, and derive a carried part from the pose that was actually written to the address space rather than the one the simulation has since moved on to. Recorded where the frame is set, not where the sample is produced, so it is the value that reached the twin. Assert it end to end: rebuild the tool centre point from the values authored into the scene and require a carried part to sit on it. The tolerance is the sampling straddle of reading two independently delivered bindings - one publish period is 43 mm at cruise - which leaves an order of magnitude between it and the defect. Two assertions in the positioning test wanted the robots a metre apart in X. They pass each other in opposite corridor lanes, where X is all but identical and the separation is entirely in Y, and before the first sample arrives both sit on the zero frame and coincide exactly. Measure the separation in the plane, once both have been seen to move. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Framing the bounds of an enclosed scene automatically puts the eye inside it: the viewport opened on a close-up of whichever robot happened to be nearest, looking through the fence. Author /Cell/OverviewCamera - the whole floor, both work tables, the docks and the controller cabinet, from 12 m out at 28 degrees - and open on the first camera the served root layer declares. A stage that authors a camera has an opinion about how it should first be seen, and the connector already had the plumbing to honour one; it was simply never given a path. --camera still overrides the choice outright. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
DynamicToolIsComposedAsync failed on both TFMs, in isolation, and a longer timeout did not help. It predates the robot-cell work. Part 5 section 9.32.2 lets only a node carrying a NodeVersion property trigger a ModelChangeEvent, and AsyncCustomNodeManager drops entries for nodes that lack one. Neither the mounted tool nor the robot it hangs off had a NodeVersion, so every addition was filtered out. Removal kept reporting, because a deleted node is no longer in the manager's index and so takes the "not mine, pass it through" branch of the same filter. That asymmetry is what made this look like flakiness. A connector starting while the tool was mounted composed it and could still deactivate it later; one starting in the six-second gap was never told the tool appeared and never composed it at all, for the life of the process. Which of the two happened depended only on the phase the fixture reached the test in - near-constant, hence a consistent failure rather than an intermittent one. Measured directly: model change events arrived only on detach, 18 s apart, and every resolve returned zero components. Enable model-change tracking on the robot, so mounting it is reportable at all. The prim now tracks the cycle: composed active for the 12 s the tool is mounted, inactive for the 6 s it is not. Assert that as a transition rather than as a momentary state - the prim seen both active and inactive across more than one full cycle. Unlike the previous check this cannot pass on a connector that only ever hears about one direction, which is precisely what the old pair of tests could not distinguish: its sibling was satisfied by the *initial* compose and never exercised reconciliation at all. Two robustness gaps this exposed in the connector: a rejected model-change monitored item was reported as success, leaving every Dynamic component frozen at whatever it resolved at start-up, and the initial compose ran before the subscription existed, so a change in between was seen by neither. Fail on the first, and re-resolve once after subscribing for the second. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Three points from the review: TransportPose and SlotOffsetsX rebuilt their array on every access, in code the choreography runs per robot per tick. Hold each as a single value instead. Made ArrayOf<double> rather than a cached double[]: a cached array in a public static hands every caller a writable reference to the one copy, so a stray write would silently move every robot's transport pose or every table slot. AdvanceSimulation returned silently when the choreographer was absent, freezing the cell with its axes at the home pose and no signal. The choreographer is a registered singleton in both hosts and everything here is driven from it, so make it a required constructor dependency: a missing registration now fails where it is made instead of looking like a stopped robot. The null guards it made unreachable are gone with it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 8cbb8cd0-f0cb-4ab0-bea2-6202fbf69485
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
MinimalRobotServerdrove its two mobile manipulators through each other. The platforms did clear — R1 and R2 trace figure-eights 1.2 m apart at closest approach, exactly asREADME.mdclaimed — but each arm reaches 1.611 m (link offsets0.260 + 0.680 + 0.670 + 0.158fromrobot.usda), so the arms swept straight through one another while the chassis politely missed. Clearance has to be argued about the deployed arm, not the footprint.Three related gaps sat alongside it: motion was open-loop (two independent sine paths with no awareness of each other), only R1 had a gripper, and nothing was ever actually picked —
PartA/PartBwere static children of a single table and the arm replayed a canned eight-pose loop unrelated to where either the robot or the table was.This replaces all of that with a coordinated transfer cycle.
Keeping them apart
The cell is divided into reservable zones, and a robot reserves the zone immediately ahead before entering it, releasing the one behind once clear — block signalling rather than a global schedule:
EndZoneA/EndZoneBCorridorEastboundCorridorWestboundDockA/DockBThree rules follow:
That last rule is not cosmetic. Without it the cell deadlocks permanently on the first handover: an idle robot standing beside its own table blocks the very delivery it is waiting for. A state trace showed R1 blocked at
x = 0.98forever while R2 sat idle holdingEndZoneB. The docks are in a southern layby rather than beside the stations, because a robot parked next to a station is close enough to foul one turning into it.Where the part actually is
A carried part's pose is computed by forward kinematics (
RobotKinematics) from the same six axis values the server publishes, not from the choreography that produced them — if the arm and the part were computed independently they would visibly drift apart.RobotArmSolversolves shoulder and elbow so the tool points down onto a slot, and is checked against that same forward kinematics, landing the tool on every slot of both tables to within 1e-6 m.Also simulated
Trapezoidal accel/cruise/decel travel with the heading following the path; speed-and-separation slowdown near the other robot; an emergency stop that now halts motion rather than only blinking the beacon; battery drain with a charging detour; seeded grip-slip faults with floor recovery; and KPIs (parts moved, cycle count, last/average cycle time, utilisation, fault count). Both robots now carry a gripper.
Two hidden couplings this surfaced
m_r1NodeIdwas keyed on "has a tool", so giving R2 a gripper silently moved the dynamic-tool demo and the speed-override command target onto R2. Now keyed on R1 explicitly.RobotPositionsDriveLiveOpenUsdSceneAsyncsampled a fixed 1.2 s window and assumed the robot was always moving. Under a duty cycle a robot legitimately stands still while gripping, dwelling or charging, so it now polls until movement is observed — the assertion's intent is that live positions reach the scene, not that the robot never stops.Invariants are asserted, not assumed
CellChoreographyTestsruns ten minutes of simulated time at 50 ms and checks on every step that:Plus the round trip completing, KPIs accumulating, the emergency stop halting every robot, dropped parts being recovered, and the battery draining and recharging.
The README documents the zoning, traffic rules and cycle, corrects the misleading clearance claim, and lists ten further realism extensions that were deliberately not built (ISO 10218 / TS 15066 speed-and-separation monitoring, scanner cones, tool changer, inspection station, conveyor infeed, gate interlock, torque telemetry, maintenance counters, time-sampled pose history, traffic priority).
Making the cycle visible
Running the sample end to end against the OpenUSD connector showed the robots driving the cycle and the arms reaching the slots while nothing was picked up or moved. The choreographer tracked part poses and gripper opening purely in memory: the address space only ever exposed the platform poses and the twelve axis angles, so a connector had nothing to subscribe to for the parts and the blocks stayed wherever the asset authored them.
A
CellTwinfolder now holds aThreeDCartesianCoordinatesposition and aThreeDOrientationheading per part, plus a jaw position per robot, bound to/Cell/Parts/PartNNand to the gripper jaws. The variables are registered before the motion device system is built - a binding can only carry aSourceNodeIdthat already exists - and sit outside the robotics subtree, because a workpiece is not a device and the robotics builder validates every node below its root as one it allocated itself.The poses are published as structured vectors, not as a matrix. A viewer composes a prim's matrix from the vector translate and rotate ops; the
matrix4dprofile is explicitly left unresolved by the reference connector, which drops a binding it cannot convert in silence. That is the shape the first attempt took, and it failed without a word in any log.The jaws needed the asset restructured. A carrier cube and a finger mesh were siblings under the gripper body, so driving the carrier would have slid it out from under the finger it is bolted to. Each jaw is now one
Xformowning both, declaring the single matrix op every live-bound prim declares, with the finger points rebased onto the jaw origin. Closed puts the finger faces on the 35 mm half-width of the block, so the gripper grips it rather than passing through it.RobotAssetContractTestsnow holds every live-bound prim to the single-matrix contract, not just the two robot mount points.No part starves
Collecting the lowest free slot meant a part that never landed in slot 0 was never collected at all: with two parts seeded on
WorkTableAthe second sat untouched for the life of the process while the robots shuttled the other one past it - which reads in the twin as the robots ignoring a block sitting in front of them. Parts are now stamped with the order in which they came to rest and the one that has waited longest is collected, which is what a real cell does with a buffer anyway.Verified end to end: all three parts circulate between both tables, the heading follows the carrying robot and resets on placement, and the jaws stroke between open (0.060) and closed (0.047) as parts are gripped and released.
The OpenUSD packages move to 0.3.0-alpha.
The part was not in the gripper
Running it end to end showed a workpiece floating clear of the jaws that were supposed to be holding it.
The platform pose and the workpiece pose leave the server as separate bindings published by different loops: positioning at 5 Hz (
UpdateIntervalMilliseconds = 200), the simulation tick at 20 Hz. A carried part rides the tool centre point, so it was being drawn against a platform up to four ticks stale. Measured against the live scene with the arm stowed — where the part-to-platform distance is fixed by construction — the part sat anywhere between 0.65 m and 0.90 m from its platform instead of at a single distance. A quarter of a metre of drift, three part widths, in front of a 96 mm jaw opening.The platform now publishes at the tick rate, and a carried part is derived from the pose that was actually written to the address space, recorded at the
SetFrameValuecall site rather than where the sample is produced. Each value was individually correct the whole time; only their relationship was wrong, which is why nothing caught it.RobotOpenUsdE2eTestsnow rebuilds the tool centre point from the values authored into the scene and asserts a carried part sits on it. It reproduced the defect at 0.49 m before the fix. The tolerance is the sampling straddle of reading two independently delivered bindings — one publish period is 43 mm at cruise — leaving an order of magnitude between it and the defect.Two assertions in the positioning test wanted the robots a metre apart in X. They pass each other in opposite corridor lanes, where X is all but identical and the separation is entirely in Y; and before the first sample arrives both platforms sit on the zero frame and coincide exactly. They now measure separation in the plane, once both have been seen to move — this was failing intermittently on net48 already.
The opening view
Framing the bounds of an enclosed scene automatically puts the eye inside it: the viewport opened on a close-up of whichever robot happened to be nearest, looking through the fence.
The cell now authors
/Cell/OverviewCamera— the whole floor, both work tables, the docks and the controller cabinet, from 12 m out at 28 degrees of elevation — and the connector opens on the first camera the served root layer declares. It already had the plumbing to honour a camera path; it was simply never given one.--camerastill overrides the choice outright, andRobotAssetContractTestsasserts the establishing shot is ordered before/Cell/TopDownCamera.The mounted gripper was never reported
DynamicToolIsComposedAsyncwas failing on both TFMs, in isolation, and a longer timeout did not help. It predates this branch — I confirmed it at58fd5e8cawith a clean worktree. It turned out not to be a flaky test at all.Part 5 §9.32.2 lets only a node carrying a
NodeVersionproperty trigger aModelChangeEvent, andAsyncCustomNodeManagerdrops entries for nodes that lack one. Neither the mounted tool nor the robot it hangs off had aNodeVersion, so every addition was filtered out. Removal kept reporting, because a deleted node is no longer in the manager's index and so takes the "not mine, pass it through" branch of the same filter.That asymmetry is what made it look like flakiness. A connector starting while the tool was mounted composed it and could still deactivate it later; one starting in the six-second gap was never told the tool appeared and never composed it at all, for the life of the process. Which happened depended only on the phase the fixture reached the test in — near-constant, hence a consistent failure rather than an intermittent one.
Measured directly before the fix: model-change events arrived only on detach, exactly 18 s apart, and every resolve returned zero components:
After enabling model-change tracking on the robot, the prim tracks the cycle —
A= composed active (12 s mounted),i= inactive (6 s detached):The test now asserts a transition, not a momentary state: the prim seen both active and inactive across more than one full cycle. Unlike the old check this cannot pass on a connector that only ever hears about one direction — which is exactly what the previous pair of tests could not distinguish, since the sibling
DynamicToolIsDeactivatedWhenDetachedAsyncwas satisfied by the initial compose and never exercised reconciliation at all.Two robustness gaps in the connector fell out of this: a rejected model-change monitored item was reported as success, leaving every
Dynamiccomponent frozen at whatever it resolved at start-up; and the initial compose ran before the subscription existed, so a change in between was seen by neither. The first now fails loudly, the second re-resolves once after subscribing.Opc.Ua.Di.Testsis now 402/403 on net48 (was 3 failures) and on net10.0, across three consecutive runs of the dynamic-tool tests on each.Related Issues
Checklist
Verification: full
dotnet build UA.slnx -m:1 /p:UseSharedCompilation=false— 0 errors, no new warnings.Opc.Ua.Di.Tests399/400 on both net10.0 and net48 (1 pre-existing skip),Opc.Ua.OpenUsd.Tests635,Opc.Ua.Robotics.Tests103.