Move jog and arm_pose onto mote_arm/cli.py's shutdown and strict parsing - #85
Merged
Conversation
…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
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.
Moves
jogandarm-poseontomote_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 callsstd::terminateandthe process exits 134 ("terminate called without an active exception") after
the tool has done its work. With no stack running to talk to,
jog(stdinclosed) and
arm-pose listeach aborted 3 of 3 runs on main, and exit 0 on3 of 3 after the fix. The 0.2 s sleep that used to mask it in
jogwentaway 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.pydid not exist on main. Task 164's brancharm/virtual-leader-teleopwas never PR'd and sits 20 commits behind. Rather than drag in 164's whole
unmerged, un-benched teleop stack, I copied
cli.pyacross byte-identical(verified with
git diff --no-index), so 164 rebasing onto this sees the fileas already-present rather than a conflict. Its
arm_pose.pyimport block willconflict, though — worth knowing before that rebase.
The parse half was the mirror image of what the task assumed. Nothing in
mote_armusesparse_known_args;arm_poseused a strictparse_argsoverthe whole command line, so ROS's arguments were not silently discarded — they
were fatal (
arm-pose list --ros-args -p use_sim_time:=truedied with"unrecognized arguments").
cli.parsecuts the--ros-args ... --block outand then parses strictly, so ROS args now pass through and a mistyped
--max_travel/--speeedis still an error. Both verified by hand.jogtakes no arguments of its own (its one input is therobot_yamlROSparameter), so it gets the shutdown half only.
arm_pose's parser moves intobuild_parser()so strictness is testable without standing a node up.Tests: new
mote_arm/test/test_cli.pypins the ROS-block stripping, thebare-
--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.pyrow;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, andits 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 lintclean, both originalreproductions re-run green (134 -> 0, 3/3 each),
--ros-argsaccepted and atypo still rejected.
cli.pyships automatically (find_packages, no setup.pychange).
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).