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

CLAUDE.md

@@ -294,6 +294,20 @@ section. Contains:
 - `jog` (CLI, `pixi run arm-jog`) — interactive per-joint jog; a *client of
   `arm_controller`* (publishes clamped single-point trajectories, limps on
   exit). It never opens the bus, so there is no contention to guard against.
+- **Every arm CLI exits and parses through `cli.py`**, because both properties
+  fail silently when hand-rolled. `cli.shutdown(node, spinner)` shuts the
+  context down, **joins the spin thread, and only then destroys the node**:
+  destroying a node `spin()` still holds aborts the interpreter (exit 134,
+  "terminate called without an active exception") *after* the tool has done its
+  work, so the run succeeds and the process still crashes — measured on `jog`
+  and `arm-pose list`, 3 of 3 runs each, with no hardware attached. This is not
+  a rare race, so a new arm CLI must not hand-roll the teardown.
+  `cli.parse(parser)` cuts the `--ros-args ... --` block out and then parses
+  strictly: `ros2 run` hands the tool ROS's arguments too, so a plain
+  `parse_args` rejects `--ros-args` outright while `parse_known_args` — the
+  usual workaround — silently discards a mistyped `--max-travel` or `--speed`
+  and drives on the default. `test_cli.py` pins both, the abort via a child
+  process's exit status since nothing in-process can catch it.
 - `arm_check` (`pixi run arm-check`) — standalone read-only enumeration/health
   + `--save-zero` calibration snapshot. Run with the driver stopped (same port).
 - **`zero` is not `home`.** `robot.yaml`'s `arm.joints[].zero` is the encoder

mote_arm/BENCH.md

@@ -323,12 +323,28 @@ motion stops at the configured `max` and the driver logs
 many times you press `+`. Repeat toward the lower limit. **This is acceptance
 criterion 2.**
 
-## Step 7 — torque-off on exit
+## Step 7 — torque-off on exit, and a clean exit
 
-In `arm-jog`, type `quit`. **Expected:** `limping arm (torque off) and
-exiting...`; the arm goes back-drivable immediately. Stop `arm` (Ctrl-C) and
-confirm it also logs a clean shutdown and leaves the arm limp. **Nothing should
-move on startup or shutdown.**
+In `arm-jog`, type `quit`. **Expected:** `limping arm (deactivating
+arm_controller) and exiting...`; the arm goes back-drivable immediately. Stop
+`arm` (Ctrl-C) and confirm it also logs a clean shutdown and leaves the arm
+limp. **Nothing should move on startup or shutdown.**
+
+Then check the exit *status*, not just the message — the arm is already limp by
+the time the process falls over, so an abort here is invisible unless looked
+for:
+
+```
+pixi run arm-jog        # 'quit' at the prompt
+echo $?                 # expect 0
+pixi run arm-pose list
+echo $?                 # expect 0
+```
+
+**Expected:** `0` from both, and no `terminate called without an active
+exception` on stderr. A `134` is the destroy-while-spinning abort (see README,
+"Exits and arguments"); it means the tool did its job and then crashed on the
+way out.
 
 ---
 

mote_arm/README.md

@@ -168,13 +168,40 @@ conversions are verified without hardware.
 | `config.py` | Parses the `arm:` section; encoder<->radian conversion + soft-limit clamping. |
 | `bus.py` | `FeetechBus` — thin `scservo_sdk` wrapper (ping, read position/health, torque, position goal, homing offset). Lazy SDK import. |
 | `control.py` | The one place that knows how to talk to `arm_controller`: single-point trajectories, and activation as the torque switch. |
+| `cli.py` | The plumbing every arm CLI shares: strict argument parsing with ROS's own arguments cut out first, and a shutdown that stops spinning before it destroys the node. Both are properties that fail silently otherwise — see "Exits and arguments" below. |
 | `arm_launch.py` (in `mote_bringup`) | Bench bring-up — the same controller_manager, URDF and `controllers.yaml` as a mission, without the lidar/camera/Nav2. `pixi run arm`. |
 | `jog` (CLI) | Interactive per-joint jog. A *client of the controller* — publishes clamped trajectories, never opens the bus. `pixi run arm-jog`. |
 | `arm_check` (tool) | Standalone enumeration + health + zero snapshot. Read-only, but opens the bus: run with the control stack stopped. `pixi run arm-check`. |
 | `calibrate.py` / `arm_calibrate` | Two-phase range calibration: sweep every joint at once, centre its zero, save limits to `$MOTE_HOME/arm.yaml`. Owns the bus: control stack stopped. `pixi run arm-calibrate`. |
 | `arm_offsets` (tool) | Read/back up/restore/set the servos' position-correction offsets. The recovery path if a calibration is interrupted. `pixi run arm-offsets`. |
 | `poses.py` / `arm_pose` | Teach and replay named poses, and narrow limits to a working envelope. `pixi run arm-pose save\|list\|go\|limits\|delete`. |
 
+## Exits and arguments
+
+Two things every arm CLI needs, both of which fail *quietly* when hand-rolled,
+so they live in `cli.py` and nowhere else.
+
+**Destroying a node that `spin()` still holds aborts the process.** The
+executor is pulled out from under itself and the interpreter calls
+`std::terminate`: exit 134, `terminate called without an active exception`,
+after the tool has already done its work. The fix is ordering — shut the
+context down, *join the spin thread*, and only then destroy — which is what
+`cli.shutdown(node, spinner)` is for. Measured on this arm's CLIs with no
+hardware attached: `jog` (stdin closed) and `arm-pose list` each aborted 3 of 3
+runs before, and exited 0 on 3 of 3 after. It is not a rare race — with no
+stack running to talk to, it reproduced every time. `test_cli.py` watches a
+child process's exit status, because nothing in-process can catch an abort.
+
+**ROS's arguments arrive mixed in with the tool's own.** `ros2 run` hands the
+executable the whole command line, so a plain `parse_args` rejects
+`--ros-args` outright (`arm-pose list --ros-args -p use_sim_time:=true` used to
+die with "unrecognized arguments") while the usual workaround,
+`parse_known_args`, silently discards anything it does not recognise — which on
+a *safety* flag means a mistyped `--max-travel` or `--speed` does nothing and
+says nothing, and the arm moves under the default instead. `cli.parse` cuts the
+`--ros-args ... --` block out first and then parses what is left strictly, so
+ROS arguments pass through and a typo is still an error.
+
 ## `zero` and `home` are different things
 
 Worth stating plainly, because the two were both called "home" until 2026-07-28

mote_arm/mote_arm/arm_pose.py

@@ -34,11 +34,10 @@
 import time
 
 import rclpy
-from rclpy.executors import ExternalShutdownException
 from rclpy.node import Node
 from sensor_msgs.msg import JointState
 
-from mote_arm import config, poses
+from mote_arm import cli, config, poses
 from mote_arm.control import ArmControl
 
 
@@ -277,7 +276,7 @@ def _stream(node: PoseClient, start: dict, goals: dict, args) -> None:
     print(f"  holding at {err:.4f} rad of residual droop")
 
 
-def main() -> None:
+def build_parser() -> argparse.ArgumentParser:
     parser = argparse.ArgumentParser(description="Teach and replay arm poses")
     sub = parser.add_subparsers(dest="cmd", required=True)
 
@@ -339,30 +338,21 @@ def main() -> None:
     )
     p_go.add_argument("--timeout", type=float, default=5.0)
     p_go.set_defaults(func=_cmd_go)
+    return parser
+
 
-    args = parser.parse_args()
+def main() -> None:
+    args = cli.parse(build_parser())
 
     rclpy.init()
     node = PoseClient()
-
-    def _spin() -> None:
-        try:
-            rclpy.spin(node)
-        except (KeyboardInterrupt, ExternalShutdownException):
-            pass
-        except Exception:  # noqa: BLE001 - context torn down by SIGINT
-            if rclpy.ok():
-                raise
-
-    threading.Thread(target=_spin, daemon=True).start()
+    spinner = cli.spin_background(node)
     try:
         args.func(node, args)
     except KeyboardInterrupt:
         print("\ninterrupted", file=sys.stderr)
     finally:
-        if rclpy.ok():
-            rclpy.shutdown()
-        node.destroy_node()
+        cli.shutdown(node, spinner)
 
 
 if __name__ == "__main__":

mote_arm/mote_arm/cli.py

@@ -0,0 +1,81 @@
+"""Command-line plumbing shared by the arm tools: arguments, and clean exits.
+
+``ros2 run`` hands the executable everything on the line, ROS's own arguments
+included, so a node with its own flags sees both. The usual answer,
+``parse_known_args``, silently discards whatever it does not recognise — which
+means a mistyped flag on a *safety* option (``--max-travel``, ``--speed-scale``)
+does nothing at all and says nothing about it.
+
+So instead: cut the ``--ros-args ... --`` block out first, then parse what is
+left strictly. A flag we do not know is then an error, as it should be.
+"""
+
+from __future__ import annotations
+
+import sys
+import threading
+from collections.abc import Sequence
+
+import rclpy
+from rclpy.executors import ExternalShutdownException
+from rclpy.node import Node
+
+
+def user_args(argv: Sequence[str] | None = None) -> list[str]:
+    """Everything on the command line that is not a ROS argument.
+
+    ``--ros-args`` opens the ROS block and a bare ``--`` closes it; that bare
+    separator is also what argparse would otherwise take as "no more options",
+    turning ``-- --camera`` into a positional it cannot place.
+    """
+    argv = list(sys.argv[1:] if argv is None else argv)
+    kept: list[str] = []
+    in_ros_block = False
+    for arg in argv:
+        if arg == "--ros-args":
+            in_ros_block = True
+        elif arg == "--":
+            in_ros_block = False
+        elif not in_ros_block:
+            kept.append(arg)
+    return kept
+
+
+def parse(parser, argv: Sequence[str] | None = None):
+    """Parse this tool's own arguments, strictly, ignoring ROS's."""
+    return parser.parse_args(user_args(argv))
+
+
+def spin_background(node: Node) -> threading.Thread:
+    """Spin a node on a worker thread while the main thread runs a CLI."""
+
+    def _spin() -> None:
+        try:
+            rclpy.spin(node)
+        except (KeyboardInterrupt, ExternalShutdownException):
+            pass
+        except Exception:  # noqa: BLE001
+            # SIGINT tears the rcl context down underneath spin(), which
+            # surfaces as different exception types depending on how the node
+            # was launched. "The context is gone" is the ordinary stop it is;
+            # anything that failed while it was still up is a real error.
+            if rclpy.ok():
+                raise
+
+    thread = threading.Thread(target=_spin, daemon=True)
+    thread.start()
+    return thread
+
+
+def shutdown(node: Node, spinner: threading.Thread) -> None:
+    """Stop spinning, then destroy the node — in that order.
+
+    Destroying a node that ``spin()`` still holds pulls it out from under the
+    executor mid-callback, and the process aborts with "terminate called
+    without an active exception" instead of exiting. Shutting the context down
+    first makes spin return; joining it is what guarantees it has.
+    """
+    if rclpy.ok():
+        rclpy.shutdown()
+    spinner.join(timeout=2.0)
+    node.destroy_node()

mote_arm/mote_arm/jog.py

@@ -22,11 +22,10 @@
 import time
 
 import rclpy
-from rclpy.executors import ExternalShutdownException
 from rclpy.node import Node
 from sensor_msgs.msg import JointState
 
-from mote_arm import config
+from mote_arm import cli, config
 from mote_arm.config import JointSpec
 from mote_arm.control import ArmControl
 
@@ -192,31 +191,15 @@ def _repl(node: JogClient) -> None:
 def main() -> None:
     rclpy.init()
     node = JogClient()
-
-    def _spin() -> None:
-        # SIGINT surfaces here as ExternalShutdownException; the REPL thread
-        # owns the exit path, so this one just stops quietly.
-        try:
-            rclpy.spin(node)
-        except (KeyboardInterrupt, ExternalShutdownException):
-            pass
-        except Exception:  # noqa: BLE001
-            # Context torn down by SIGINT; a real error is one that happened
-            # while the context was still valid.
-            if rclpy.ok():
-                raise
-
-    spin = threading.Thread(target=_spin, daemon=True)
-    spin.start()
+    spinner = cli.spin_background(node)
     try:
         _repl(node)
     except KeyboardInterrupt:
         pass
     finally:
         print("\nlimping arm (deactivating arm_controller) and exiting...")
         node.arm.set_holding(False)
-        rclpy.shutdown()
-        node.destroy_node()
+        cli.shutdown(node, spinner)
 
 
 if __name__ == "__main__":

mote_arm/test/test_cli.py

@@ -0,0 +1,92 @@
+"""The shared CLI plumbing: strict argument parsing, and exits that are exits.
+
+Both properties fail *silently* if they regress, which is why they are pinned
+here. A mistyped safety flag that argparse drops on the floor changes nothing
+and says nothing; and a node destroyed while ``spin()`` still holds it aborts
+the process after the useful work is done, so the operator sees a crash on a
+run that in fact succeeded.
+"""
+
+import os
+import random
+import subprocess
+import sys
+import textwrap
+
+import pytest
+
+from mote_arm import cli
+from mote_arm.arm_pose import build_parser
+
+
+def test_user_args_drops_the_ros_block():
+    assert cli.user_args(["go", "home", "--ros-args", "-p", "x:=1"]) == ["go", "home"]
+
+
+def test_bare_separator_closes_the_ros_block():
+    """``--`` ends the ROS block; what follows is ours again.
+
+    argparse would read that bare ``--`` as "no more options" and turn a flag
+    after it into a positional it cannot place, so it must not survive.
+    """
+    argv = ["go", "home", "--ros-args", "-p", "x:=1", "--", "--yes"]
+    assert cli.user_args(argv) == ["go", "home", "--yes"]
+
+
+def test_no_ros_block_is_left_alone():
+    assert cli.user_args(["go", "home", "--yes"]) == ["go", "home", "--yes"]
+
+
+def test_parse_accepts_a_safety_flag_past_a_ros_block():
+    args = cli.parse(
+        build_parser(),
+        ["go", "home", "--max-travel", "1.25", "--ros-args", "-p", "x:=1"],
+    )
+    assert args.name == "home"
+    assert args.max_travel == 1.25
+
+
+@pytest.mark.parametrize("flag", ["--max_travel", "--maxtravel", "--speeed"])
+def test_a_mistyped_safety_flag_is_an_error(flag):
+    """Not a warning, and above all not silence: the arm would move anyway."""
+    with pytest.raises(SystemExit) as exc:
+        cli.parse(build_parser(), ["go", "home", flag, "0.9"])
+    assert exc.value.code != 0
+
+
+# Destroying a node the executor still holds aborts the interpreter itself
+# (SIGABRT, "terminate called without an active exception"), so no in-process
+# assertion can catch it — the test has to watch a child's exit status.
+CHILD = """\
+import threading
+import rclpy
+from rclpy.node import Node
+from mote_arm import cli
+
+rclpy.init()
+node = Node("cli_shutdown_probe")
+node.create_timer(0.001, lambda: None)
+spinner = cli.spin_background(node)
+threading.Event().wait(0.4)
+cli.shutdown(node, spinner)
+print("clean")
+"""
+
+
+def test_shutdown_exits_cleanly():
+    env = dict(os.environ)
+    env["ROS_DOMAIN_ID"] = str(random.randint(60, 100))
+    env["PYTHONPATH"] = os.pathsep.join(sys.path)
+    done = subprocess.run(
+        [sys.executable, "-c", textwrap.dedent(CHILD)],
+        capture_output=True,
+        text=True,
+        timeout=60,
+        env=env,
+    )
+    assert done.returncode == 0, (
+        f"exit {done.returncode} (-6/134 is the abort this guards against)\n"
+        f"{done.stderr}"
+    )
+    assert "clean" in done.stdout
+    assert "terminate called" not in done.stderr