Skip to content

Move jog and arm_pose onto mote_arm/cli.py's shutdown and strict parsing - #85

Merged
MJohnson459 merged 1 commit into
mainfrom
arm/cli-shutdown-jog-pose
Jul 31, 2026
Merged

Move jog and arm_pose onto mote_arm/cli.py's shutdown and strict parsing#85
MJohnson459 merged 1 commit into
mainfrom
arm/cli-shutdown-jog-pose

Conversation

@MJohnson459

Copy link
Copy Markdown
Contributor

Moves jog and arm-pose onto mote_arm/cli.py's shutdown and strict parsing.
Branch arm/cli-shutdown-jog-pose, one commit (ce75388), committed not pushed.

The bug is not latent — it fires every time. Both tools destroy their node
while rclpy.spin() still holds it; the interpreter calls std::terminate and
the process exits 134 ("terminate called without an active exception") after
the tool has done its work. With no stack running to talk to, jog (stdin
closed) and arm-pose list each aborted 3 of 3 runs on main, and exit 0 on
3 of 3 after the fix. The 0.2 s sleep that used to mask it in jog went
away with the ros2_control fold (#74), so nothing has been hiding it since — it
just was not looked for, since the arm is already limp by then.

cli.py did not exist on main. Task 164's branch arm/virtual-leader-teleop
was never PR'd and sits 20 commits behind. Rather than drag in 164's whole
unmerged, un-benched teleop stack, I copied cli.py across byte-identical
(verified with git diff --no-index), so 164 rebasing onto this sees the file
as already-present rather than a conflict. Its arm_pose.py import block will
conflict, though — worth knowing before that rebase.

The parse half was the mirror image of what the task assumed. Nothing in
mote_arm uses parse_known_args; arm_pose used a strict parse_args over
the whole command line, so ROS's arguments were not silently discarded — they
were fatal (arm-pose list --ros-args -p use_sim_time:=true died with
"unrecognized arguments"). cli.parse cuts the --ros-args ... -- block out
and then parses strictly, so ROS args now pass through and a mistyped
--max_travel / --speeed is still an error. Both verified by hand.
jog takes no arguments of its own (its one input is the robot_yaml ROS
parameter), so it gets the shutdown half only. arm_pose's parser moves into
build_parser() so strictness is testable without standing a node up.

Tests: new mote_arm/test/test_cli.py pins the ROS-block stripping, the
bare--- case, strict rejection of mistyped safety flags, and the clean exit —
the last by watching a child process's exit status, since nothing
in-process can catch an abort.

Docs: README gains an "Exits and arguments" section and a cli.py row;
CLAUDE.md gains the rule that every arm CLI exits and parses through cli.py;
BENCH.md step 7 now says to check echo $? rather than only the message, and
its expected wording is corrected (it was still quoting the pre-#74 "torque
off", which the code has not printed since).

Verified: 178 mote_arm tests pass, pixi run lint clean, both original
reproductions re-run green (134 -> 0, 3/3 each), --ros-args accepted and a
typo still rejected. cli.py ships automatically (find_packages, no setup.py
change).

Not verified — the remaining gate: nothing was run on the real arm. BENCH.md
steps 5-7 want a re-run to confirm the clean-shutdown property holds with a
stack actually owning the bus (the reproductions above ran with no stack, which
is the easier case for the race, not the harder one).

…t today)

Both tools destroy their node while rclpy.spin() still holds it, which pulls
the executor out from under itself: the interpreter calls std::terminate and
the process exits 134 with "terminate called without an active exception" —
after the tool has already done its work. So the run succeeds and the operator
sees a crash.

This was believed latent. It is not. With no stack running to talk to, `jog`
(stdin closed) and `arm-pose list` each aborted 3 of 3 runs on this branch's
parent, and exit 0 on 3 of 3 after. The 0.2 s sleep that used to mask it in
jog went away with the ros2_control fold (#74), so nothing has been hiding it
since; it simply had not been looked for, because the arm is already limp by
then and the message scrolls past.

The fix is ordering, and it belongs in one place rather than in each main():
shut the context down, *join the spin thread*, and only then destroy.
cli.spin_background/cli.shutdown are taken verbatim from the arm/virtual-leader
-teleop branch, where they were written for the same bug in episode_record and
have not landed yet — byte-identical, so that branch rebases onto this without
a conflict.

The argument half turned out to be the mirror image of what was expected.
Neither tool uses parse_known_args (nothing in mote_arm does); arm_pose uses a
strict parse_args over the whole command line, so ROS's own arguments are not
discarded, they are fatal — `arm-pose list --ros-args -p use_sim_time:=true`
dies with "unrecognized arguments". cli.parse cuts the `--ros-args ... --`
block out first and then parses strictly, so ROS arguments pass through and a
mistyped safety flag (`--max_travel`, `--speeed`) is still an error rather than
silently dropped onto the default. jog takes no arguments of its own — its one
input is the robot_yaml ROS parameter — so there is nothing there to parse; it
gets the shutdown half only. arm_pose's parser moves into build_parser() so the
strictness is testable without standing a node up.

test_cli.py pins both properties. The abort is checked by watching a child
process's exit status, because nothing in-process can catch an abort.

Verified: 178 mote_arm tests pass, lint clean, and the two reproductions above
re-run green. Not verified: anything on the real arm — BENCH.md steps 5-7 want
a re-run, and step 7 now says to check the exit status rather than only the
message (it was also still quoting the pre-#74 "torque off" wording).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013uSB4sBqvYnsZRAEqLR4U8
@MJohnson459
MJohnson459 merged commit 9284d37 into main Jul 31, 2026
3 checks passed
@MJohnson459
MJohnson459 deleted the arm/cli-shutdown-jog-pose branch July 31, 2026 21:20
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.

1 participant