Rewrite as a dora 1.0 service node #2

Merged
cristhian merged 9 commits from refactor/dora-1.0-services into main 2026-09-15 16:37:59 +00:00
Owner

Summary

Brings the node up to the dora 1.0 contract and replaces its command/status JSON pair
with dora's own service pattern: one request, one response, correlated by the
request_id dora puts in the message metadata.

The FastAPI REST server and web UI are gone — the node is a dataflow node and nothing
else, and its dependency set is the arm's SDK rather than a web stack. Both remain
reachable at tag v0.1.2.

The driver layer is ported from
monolitic-littlehand-python
(littlehand/robot/), which was written against this same controller. Fixes made there
belong here, and vice versa.

Related Issues

None — the repository has no issue tracker entries, and this was agreed rather than
filed retroactively.

Changes

Services, not commands. Eleven services: move_to, move_joints, home,
home_and_clear, stop, clear, recover, freedrive, grasp, release,
get_state. Payloads are Arrow structs rather than JSON strings, and the correlation id
moves from the payload into the message metadata, where dora's own pattern puts it.

Blocking and non-blocking motions. A motion takes a wait flag: hold the caller
until the arm stops, or answer at once with an acknowledgement. Either way it is exactly
one response per request — with wait: false that response is the acknowledgement, and
how the motion ended is reported on motion_state, never as a second response.

The motion lifecycle is the arm's state, not an action. The arm is one resource, so
there is never more than one motion: a new one preempts the one in play, and a state
describes that exactly where a goal/feedback/result pattern would be modelling
concurrency that cannot physically exist. motion_state is keyed by request_id and
publishes accepted before the control thread reaches the motion — both to close one
race, where a caller polling a bare moving/idle flag right after its request would
read "not moving" and conclude the motion had already finished.

Three threads, because the SDK blocks. The control thread carries out one job at a
time and may block. The reporter thread only ever reads the controller's report cache, so
tcp_pose keeps flowing through a ten-second move. The stopper thread exists because a
stop fits nowhere else: it must not queue behind the motion it aborts, and the SDK's
emergency_stop re-commands the controller for up to three seconds — far too long to
stop routing a dataflow. The event loop never calls anything that can block.

Driver safety brought across from littlehand. Typed failures, live error-code reads
(the cached one is stale exactly when it matters), arrival confirmed rather than assumed,
and a move refused before it is commanded if its straight-line path leaves the joint
envelope.

Lifecycle as a dora node. The node joins the dataflow before the arm threads start,
so INIT_MODE cannot move a robot before the process has joined — and if joining then
failed, it would have moved with nobody watching. A restart leaves the arm where it is
rather than driving it home unasked. Shutdown halts the arm first: a motion lives in the
controller, not in this process, so dropping the connection does not abort it.

Node contract. dora-node.yml manifest with typed telemetry outputs;
request/response deliberately untyped, because dora treats a port carrying
request_id as pattern-correlated and skips type checks on it. strict_types: true
dataflow built from the checkout, plus a worked example in examples/caller.py.

CI. dora validate, ruff, pytest and markdownlint on Gitea Actions. None of them
need a robot.

One bug worth calling out

The reporter's _running event was set while running and waited on as its sleep, so
every wait(interval) returned at once. TELEMETRY_INTERVAL_MS had no effect and the
thread span at 1,403,398 arm reads per second, burning a core while behaving
correctly in every visible way; the reconnect backoff did not back off either. Measured
at 9 reads/s after the fix. test_the_reporter_honours_the_telemetry_interval guards it.

Fixed after review

Two bugs found in review, each with a regression test that fails without the fix.

A zero-velocity jog reported a running motion as idle (608e220). start_jog restored
ready for any zero velocity without checking whether a motion owned the arm, so a
teleop source streaming zero-velocity ticks during a move published moving → ready
mid-motion — and the closing guard in _run_motion then saw ready rather than moving
and never republished, so a caller watching robot_state concluded the motion had
finished early. Every "the arm has stopped" transition now goes through
_idle_unless_busy, which reports ready only when no motion is running, no jog is
driving the arm, and the state has not already left moving for unavailable or
recovering. That also closes the mirror-image case: a jog that preempted a motion left
_jogging true, and the motion's closing guard published ready over the jog that had
taken the arm over from it. motion_state was correct throughout, in both directions —
this was the coarse robot_state channel only.

Requests queued while the arm was down were never answered (4427da9). _run_control
did not drain the job queue until the arm had been reached, so a request arriving during
an outage waited for a connection that might never come — breaking the
one-response-per-request rule, and circularly so: requests are queued before the arm is
reached because recover is what a caller sends then, yet recover is served by the
control thread and so was the one service that could never run while down. The wait
between connection attempts now serves the queue instead of sleeping through it.
Non-motion services are tried on the arm, which is what lets recover and clear reach
a controller that answers while refusing to serve. A motion is refused rather than held:
the arm is somewhere unknown and nobody is watching, so a held motion would fire
unattended the moment the link came back.

Validation

  • Tests pass — 86 tests, run several times for flakiness; no robot, dataflow or daemon needed
  • dora validate dataflow.yml clean under strict_types: true (dora-cli 1.0.1)
  • ruff check . and markdownlint clean
  • Smoke-tested against an unreachable controller: degrades to unavailable, retries on one reused SDK handle, threads stable, clean shutdown
  • Measured: request_stop returns in 0.0000 s (was up to 3 s inline); reporter honours its interval

Not verified — no hardware was connected

  • That emergency_stop() followed by clear() leaves the controller accepting the next
    motion. The clear() before a post-preemption move is deliberately belt-and-braces:
    set_position already does set_mode(0)/set_state(0), but I did not want to depend
    on that being enough. First thing to check on real hardware.
  • That the xArm controller keeps executing a queued motion when the TCP connection drops.
    The shutdown-halts-the-arm change assumes it does.

Breaking changes

Not drop-in from v0.2.x. command/status → request/response; JSON strings → Arrow
structs; action → service; vacuum_on/vacuum_off → grasp/release; jog is its
own input; no started message. README carries the full migration notes, including the
v0.1.x ones (web UI removal, DEFAULT_UNITS, tool-tip poses).

# Summary Brings the node up to the dora 1.0 contract and replaces its `command`/`status` JSON pair with dora's own **service pattern**: one request, one response, correlated by the `request_id` dora puts in the message metadata. The FastAPI REST server and web UI are gone — the node is a dataflow node and nothing else, and its dependency set is the arm's SDK rather than a web stack. Both remain reachable at tag `v0.1.2`. The driver layer is ported from [monolitic-littlehand-python](https://gitea.liria.cl/Littlehand/monolitic-littlehand-python) (`littlehand/robot/`), which was written against this same controller. Fixes made there belong here, and vice versa. ## Related Issues None — the repository has no issue tracker entries, and this was agreed rather than filed retroactively. ## Changes **Services, not commands.** Eleven services: `move_to`, `move_joints`, `home`, `home_and_clear`, `stop`, `clear`, `recover`, `freedrive`, `grasp`, `release`, `get_state`. Payloads are Arrow structs rather than JSON strings, and the correlation id moves from the payload into the message metadata, where dora's own pattern puts it. **Blocking and non-blocking motions.** A motion takes a `wait` flag: hold the caller until the arm stops, or answer at once with an acknowledgement. Either way it is exactly one response per request — with `wait: false` that response *is* the acknowledgement, and how the motion ended is reported on `motion_state`, never as a second response. **The motion lifecycle is the arm's state, not an action.** The arm is one resource, so there is never more than one motion: a new one preempts the one in play, and a state describes that exactly where a goal/feedback/result pattern would be modelling concurrency that cannot physically exist. `motion_state` is keyed by `request_id` and publishes `accepted` before the control thread reaches the motion — both to close one race, where a caller polling a bare `moving`/`idle` flag right after its request would read "not moving" and conclude the motion had already finished. **Three threads, because the SDK blocks.** The control thread carries out one job at a time and may block. The reporter thread only ever reads the controller's report cache, so `tcp_pose` keeps flowing through a ten-second move. The stopper thread exists because a stop fits nowhere else: it must not queue behind the motion it aborts, and the SDK's `emergency_stop` re-commands the controller for up to three seconds — far too long to stop routing a dataflow. The event loop never calls anything that can block. **Driver safety brought across from littlehand.** Typed failures, live error-code reads (the cached one is stale exactly when it matters), arrival confirmed rather than assumed, and a move refused before it is commanded if its straight-line path leaves the joint envelope. **Lifecycle as a dora node.** The node joins the dataflow *before* the arm threads start, so `INIT_MODE` cannot move a robot before the process has joined — and if joining then failed, it would have moved with nobody watching. A restart leaves the arm where it is rather than driving it home unasked. Shutdown halts the arm first: a motion lives in the controller, not in this process, so dropping the connection does not abort it. **Node contract.** `dora-node.yml` manifest with typed telemetry outputs; `request`/`response` deliberately untyped, because dora treats a port carrying `request_id` as pattern-correlated and skips type checks on it. `strict_types: true` dataflow built from the checkout, plus a worked example in `examples/caller.py`. **CI.** `dora validate`, `ruff`, `pytest` and markdownlint on Gitea Actions. None of them need a robot. ### One bug worth calling out The reporter's `_running` event was *set* while running and waited on as its sleep, so every `wait(interval)` returned at once. `TELEMETRY_INTERVAL_MS` had no effect and the thread span at **1,403,398 arm reads per second**, burning a core while behaving correctly in every visible way; the reconnect backoff did not back off either. Measured at 9 reads/s after the fix. `test_the_reporter_honours_the_telemetry_interval` guards it. ### Fixed after review Two bugs found in review, each with a regression test that fails without the fix. **A zero-velocity jog reported a running motion as idle** (608e220). `start_jog` restored `ready` for any zero velocity without checking whether a motion owned the arm, so a teleop source streaming zero-velocity ticks during a move published `moving` → `ready` mid-motion — and the closing guard in `_run_motion` then saw `ready` rather than `moving` and never republished, so a caller watching `robot_state` concluded the motion had finished early. Every "the arm has stopped" transition now goes through `_idle_unless_busy`, which reports `ready` only when no motion is running, no jog is driving the arm, and the state has not already left `moving` for `unavailable` or `recovering`. That also closes the mirror-image case: a jog that preempted a motion left `_jogging` true, and the motion's closing guard published `ready` over the jog that had taken the arm over from it. `motion_state` was correct throughout, in both directions — this was the coarse `robot_state` channel only. **Requests queued while the arm was down were never answered** (4427da9). `_run_control` did not drain the job queue until the arm had been reached, so a request arriving during an outage waited for a connection that might never come — breaking the one-response-per-request rule, and circularly so: requests are queued before the arm is reached because `recover` is what a caller sends then, yet `recover` is served by the control thread and so was the one service that could never run while down. The wait between connection attempts now serves the queue instead of sleeping through it. Non-motion services are tried on the arm, which is what lets `recover` and `clear` reach a controller that answers while refusing to serve. A motion is refused rather than held: the arm is somewhere unknown and nobody is watching, so a held motion would fire unattended the moment the link came back. ## Validation - [x] Tests pass — 86 tests, run several times for flakiness; no robot, dataflow or daemon needed - [x] `dora validate dataflow.yml` clean under `strict_types: true` (dora-cli 1.0.1) - [x] `ruff check .` and markdownlint clean - [x] Smoke-tested against an unreachable controller: degrades to `unavailable`, retries on one reused SDK handle, threads stable, clean shutdown - [x] Measured: `request_stop` returns in 0.0000 s (was up to 3 s inline); reporter honours its interval ### Not verified — no hardware was connected - That `emergency_stop()` followed by `clear()` leaves the controller accepting the next motion. The `clear()` before a post-preemption move is deliberately belt-and-braces: `set_position` already does `set_mode(0)`/`set_state(0)`, but I did not want to depend on that being enough. **First thing to check on real hardware.** - That the xArm controller keeps executing a queued motion when the TCP connection drops. The shutdown-halts-the-arm change assumes it does. ## Breaking changes Not drop-in from v0.2.x. `command`/`status` → `request`/`response`; JSON strings → Arrow structs; `action` → `service`; `vacuum_on`/`vacuum_off` → `grasp`/`release`; `jog` is its own input; no `started` message. README carries the full migration notes, including the v0.1.x ones (web UI removal, `DEFAULT_UNITS`, tool-tip poses).
cristhian added 6 commits 2026-09-12 14:47:34 +00:00
Brings across the robot layer from monolitic-littlehand-python, which was
written against this same controller. The point of it is that a command
either moved the arm or raised: the SDK returns a code for a motion the
controller quietly refused, wait_move calls that refusal finished, and
arm.error_code is the report stream's stale copy.

So errors are read live, arrival is confirmed rather than assumed, and a
move whose straight-line path leaves the joint envelope is refused before
it is commanded. Failures are typed, because the distinctions decide the
response: an unreachable pose is not an unavailable arm, and a joint past
its limit needs a motion nobody should start without warning the room.

read_report is the deliberate exception to all of it: a report-cache read
that costs no round trip and cannot block, which is the only read that
works while a motion is in flight.
Replaces the command/status JSON pair with dora's own service pattern:
one request, one response, correlated by the request_id dora puts in the
message metadata. Motions take a wait flag, and their lifecycle is
published as the arm's state on motion_state rather than as a per-goal
stream — the arm is one resource, so a new motion preempts the one in
play and a state describes that exactly.

motion_state is keyed by request_id and publishes accepted before the
control thread reaches the motion. Both exist to close one race: a caller
polling a bare moving/idle flag right after its request would read "not
moving" and conclude the motion had already finished.

Three threads, because the SDK blocks. The control thread carries out one
job at a time and may block; the reporter thread only ever reads the
report cache, so telemetry keeps flowing through a ten-second move; the
stopper thread exists because a stop fits nowhere else — it must not
queue behind the motion it aborts, and emergency_stop re-commands the
controller for up to three seconds, which is far too long to stop routing
a dataflow.

The event loop never calls anything that can block, get_state included:
that one is allowed there only because it reads the cache. The node is
built before the arm threads start, so INIT_MODE cannot move a robot
before the process has joined the dataflow, and a restart leaves the arm
where it is rather than driving it home unasked. Shutdown halts the arm
first — a motion lives in the controller, and dropping the connection
does not abort it.
The manifest is the only contract a consumer has, since no source package
is shared between node repositories, and dora validate checks a dataflow
against it without running the node — so CI needs no robot.

request and response are deliberately untyped. They carry request_id, and
dora defines that as the marker for a pattern-correlated port where
several Arrow schemas legitimately flow, one per service; the runtime
skips type checks on those messages, so annotating them would claim a
contract neither side keeps.

examples/ carries its own manifest because dora resolves a node's
manifest by walking up from its path, and a manifest applies only when
<manifest dir>/<entrypoint> names the same file as the dataflow's path.
Without it the example caller would be matched against the arm node's
manifest and strict validation would fail on the entrypoint mismatch.
dora-rs 1.0.1 requires Python 3.11+. Drops the numpy<2 pin, which the
littlehand lockfile shows is unnecessary with the current xArm SDK, and
moves to hatchling with a uv lockfile that CI installs with --locked so a
dependency change that was never locked cannot reach main.

Removes scripts/install.sh: uv sync is the whole install now.
Replaces the GitHub markdownlint workflow with a Gitea one covering all
four gates, matching dora-zed and dora-plot. None of them need a robot:
dora validate runs no node, and the tests need neither a dataflow nor a
daemon.
docs: rewrite README and AGENTS for the service node
validate / validate (pull_request) Successful in 58m45s
validate / test (pull_request) Successful in 58m51s
validate / markdownlint (pull_request) Successful in 58m44s
412aeac955
Documents the eleven services, the wait flag, motion_state and
preemption, and the lifecycle guarantees around restart and shutdown.

AGENTS.md records the two rules that are easy to break by accident: the
stopping event must stay set-to-stop, because an event that is set while
running makes every wait() return at once and turns each cadence into a
busy loop; and every request gets exactly one response, since a caller
has no other way to know its request finished.
Author
Owner

/bot review

/bot review
bot reviewed 2026-09-15 15:40:05 +00:00
bot left a comment
Owner

Summary

This rewrite moves the arm behind a dedicated control/reporter/stopper thread trio and a request_id-correlated service protocol, which is a sound structure — but the state machine publishes a false idle during motion, and requests accepted while the arm is down are never answered at all.

Findings

  • dora_ulite6/control.py:237 — start_jog publishes READY for any zero-velocity jog (if not moving: self._set_state(READY)) without checking whether a motion owns the arm. A teleop source streaming zero-velocity ticks while a move runs makes the node publish moving → ready mid-motion — the exact "an arm in motion must never look idle" invariant this PR's own tests assert — and when the motion ends, _run_motion's closing if self._state == MOVING guard sees READY and never republishes, so callers polling state to follow the motion conclude it finished early. The zero-jog transition should only restore READY when it was the jogging flag that was set (mirror of _leave_jog).
  • dora_ulite6/control.py:310 — _run_control gates on if not self._started: self._start_up(); continue and therefore never drains self._jobs while the arm is UNAVAILABLE, so every request queued during the outage — motions included — gets no response ever, violating the one-response-per-request rule main.py:108 states as absolute ("never silence"). It is also circular: main.py:110 justifies queueing pre-reach requests because "recover and get_state are precisely what a caller sends then", yet IMMEDIATE is only {"get_state"} (services.py:160), so recover is the one thing that can never be serviced while the arm is down. Worse, a queued motion from before the outage fires unattended the moment the link returns. Either drain the queue while not started (answering with a refusal/queued outcome) or answer every pending call when entering UNAVAILABLE.

Recommendation

Request changes — fix the jog-overshadows-motion state transition and the unanswered-while-unavailable queue before merge.

## Summary This rewrite moves the arm behind a dedicated control/reporter/stopper thread trio and a request_id-correlated service protocol, which is a sound structure — but the state machine publishes a false idle during motion, and requests accepted while the arm is down are never answered at all. ## Findings - **`dora_ulite6/control.py:237`** — `start_jog` publishes READY for any zero-velocity jog (`if not moving: self._set_state(READY)`) without checking whether a motion owns the arm. A teleop source streaming zero-velocity ticks while a move runs makes the node publish `moving` → `ready` mid-motion — the exact "an arm in motion must never look idle" invariant this PR's own tests assert — and when the motion ends, `_run_motion`'s closing `if self._state == MOVING` guard sees READY and never republishes, so callers polling state to follow the motion conclude it finished early. The zero-jog transition should only restore READY when it was the jogging flag that was set (mirror of `_leave_jog`). - **`dora_ulite6/control.py:310`** — `_run_control` gates on `if not self._started: self._start_up(); continue` and therefore never drains `self._jobs` while the arm is UNAVAILABLE, so every request queued during the outage — motions included — gets **no response ever**, violating the one-response-per-request rule `main.py:108` states as absolute ("never silence"). It is also circular: `main.py:110` justifies queueing pre-reach requests because "`recover` and `get_state` are precisely what a caller sends then", yet `IMMEDIATE` is only `{"get_state"}` (`services.py:160`), so `recover` is the one thing that can never be serviced while the arm is down. Worse, a queued motion from before the outage fires unattended the moment the link returns. Either drain the queue while not started (answering with a refusal/queued outcome) or answer every pending call when entering UNAVAILABLE. ## Recommendation **Request changes** — fix the jog-overshadows-motion state transition and the unanswered-while-unavailable queue before merge.
cristhian added 3 commits 2026-09-15 16:34:18 +00:00
start_jog restored READY for any zero velocity, without checking whether a
motion owned the arm. A teleop source streaming zero-velocity ticks during a
move published moving -> ready mid-motion, and the closing guard in _run_motion
then saw READY rather than MOVING and never republished, so a caller watching
robot_state concluded the motion had finished early.

Route every "the arm has stopped" transition through _idle_unless_busy, which
reports READY only when no motion is running, no jog is driving the arm, and the
state has not already left MOVING for unavailable or recovering.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_run_control never drained the job queue while the arm had not been reached, so
every request that arrived during an outage waited for a connection that might
never come. That breaks the one-response-per-request rule the node states as
absolute, and it was circular: requests are queued before the arm is reached
because recover is what a caller sends then, yet recover is served by the
control thread and so was the one service that could never run while down.

Serve the queue during the wait between connection attempts. Non-motion services
are tried on the arm, which is what lets recover and clear reach a controller
that answers while refusing to serve. A motion is refused rather than held: the
arm is somewhere unknown and nobody is watching, so a held motion would fire
unattended the moment the link came back.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
docs: update the test count
validate / markdownlint (pull_request) Successful in 58m43s
validate / validate (pull_request) Successful in 58m48s
validate / test (pull_request) Successful in 58m55s
8cd4db93f1
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Author
Owner

Both findings were real. Fixed in 608e220 and 4427da9, each with a regression test that fails without the fix.

start_jog publishing a false idle — confirmed as described. Every "the arm has stopped" transition now goes through _idle_unless_busy, which reports ready only when no motion is running, no jog is driving the arm, and the state has not already left moving for unavailable or recovering. That replaces the closing if self._state == MOVING guard in _run_motion as well.

One clarification on the finding's reach: motion_state was never affected — _finish_motion publishes done/failed keyed by request_id regardless of what robot_state said. The corruption was confined to the coarse robot_state channel, which is still exactly the channel the "never look idle" invariant is about.

The fix also closes the mirror-image case the review did not name: a jog that preempted a motion left _jogging true, and the motion's own closing guard then published ready over the jog that had taken the arm over from it.

Requests unanswered while the arm is down — confirmed, including the circularity. The wait between connection attempts now serves the queue (_serve_while_down) instead of sleeping through it. Non-motion services are tried on the arm, which is what lets recover and clear reach a controller that answers while refusing to serve, so the justification in main.py for queueing pre-reach requests is now true rather than circular. A motion is refused there rather than held, on the review's own reasoning: the arm is somewhere unknown and nobody is watching, so a held motion would fire unattended the moment the link returned.

Suite is 86 tests, run three times for flakiness. ruff and markdownlint clean. Still no hardware — the two items under "Not verified" stand unchanged.

Both findings were real. Fixed in 608e220 and 4427da9, each with a regression test that fails without the fix. **`start_jog` publishing a false idle** — confirmed as described. Every "the arm has stopped" transition now goes through `_idle_unless_busy`, which reports `ready` only when no motion is running, no jog is driving the arm, and the state has not already left `moving` for `unavailable` or `recovering`. That replaces the closing `if self._state == MOVING` guard in `_run_motion` as well. One clarification on the finding's reach: `motion_state` was never affected — `_finish_motion` publishes `done`/`failed` keyed by `request_id` regardless of what `robot_state` said. The corruption was confined to the coarse `robot_state` channel, which is still exactly the channel the "never look idle" invariant is about. The fix also closes the mirror-image case the review did not name: a jog that preempted a motion left `_jogging` true, and the motion's own closing guard then published `ready` over the jog that had taken the arm over from it. **Requests unanswered while the arm is down** — confirmed, including the circularity. The wait between connection attempts now serves the queue (`_serve_while_down`) instead of sleeping through it. Non-motion services are tried on the arm, which is what lets `recover` and `clear` reach a controller that answers while refusing to serve, so the justification in `main.py` for queueing pre-reach requests is now true rather than circular. A motion is refused there rather than held, on the review's own reasoning: the arm is somewhere unknown and nobody is watching, so a held motion would fire unattended the moment the link returned. Suite is 86 tests, run three times for flakiness. `ruff` and markdownlint clean. Still no hardware — the two items under "Not verified" stand unchanged.
cristhian merged commit f29265a860 into main 2026-09-15 16:37:59 +00:00
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: liria/dora-ulite6#2