From d1f0b3e9a3227859ef495a1b65965cd4b7656188 Mon Sep 17 00:00:00 2001 From: Griswald Brooks Date: Sat, 12 Sep 2026 16:43:09 -0400 Subject: [PATCH] feat(so101_base_config): bound Feetech bus write retries in calibrate_so101 Bench evidence (2026-09-12, WCH CH343 adapters, STS3215 servos): stock lerobot_calibrate failed at connect with "Incorrect status packet" on a WRITE to a far-end servo, while a 300-read stress test on the same bus was clean. LeRobot's MotorsBus._write/_sync_write take num_retry but the CLI never sets it. calibrate_so101.py now runs lerobot_calibrate through a shim that floors num_retry at 5 (--bus-write-retries to change it, 0 to run the stock CLI unmodified). Also updates the README's "Known gaps" P_Coefficient note check: that paragraph already reflects the PR 942 gain-restore fix, so no edit was needed there - left as-is. --- src/so101_base_config/README.md | 11 ++++ .../script/calibrate_so101.py | 63 +++++++++++++++++-- .../test/test_calibrate_so101.py | 50 ++++++++++++++- 3 files changed, 119 insertions(+), 5 deletions(-) diff --git a/src/so101_base_config/README.md b/src/so101_base_config/README.md index 656016596..f6eeb7790 100644 --- a/src/so101_base_config/README.md +++ b/src/so101_base_config/README.md @@ -571,6 +571,17 @@ back in). If `lerobot` is not importable, the script fails with a message naming the venv command above rather than trying to `pip install` anything itself. +The bench's WCH CH343 adapters can drop the first reply to a bus WRITE (the +same failure mode as the driver's own first-packet retry, see *Known gaps in +the driver* above), which surfaces as `lerobot_calibrate` failing to connect +with `Incorrect status packet`. By default `script/calibrate_so101.py` runs +`lerobot_calibrate` through a small shim that monkeypatches LeRobot's own +`MotorsBus._write`/`_sync_write` to floor their retry count at 5 before +calling LeRobot's unmodified CLI — nothing else about the calibration changes, +since the retry happens inside LeRobot's own bus layer. Pass +`--bus-write-retries N` to change the floor, or `--bus-write-retries 0` to run +the stock `lerobot_calibrate` invocation unwrapped. + LeRobot's calibration `connect()` writes P_Coefficient=16 into every follower servo as a side effect of connecting, not a deliberate SO-101 tuning choice — the STS3215 factory default is 32. At P=16, the loaded joints (especially diff --git a/src/so101_base_config/script/calibrate_so101.py b/src/so101_base_config/script/calibrate_so101.py index 04abbd5ee..2641b39ea 100755 --- a/src/so101_base_config/script/calibrate_so101.py +++ b/src/so101_base_config/script/calibrate_so101.py @@ -65,6 +65,38 @@ CALIBRATION_DIR = Path.home() / ".cache" / "huggingface" / "lerobot" / "calibration" +# Bench evidence (2026-09-12, WCH CH343 adapters, STS3215 servos): the stock +# lerobot_calibrate failed twice at connect with "Incorrect status packet" on +# a WRITE to a far-end servo, while a 300-read stress test on the same bus was +# clean. LeRobot's MotorsBus._write/_sync_write take num_retry but the CLI +# never sets it. This shim monkeypatches both to floor num_retry at N before +# running the stock CLI unmodified otherwise; see the README's "Calibrating a +# new arm" section. +BUS_WRITE_RETRY_SHIM = """ +import sys +import lerobot.motors.motors_bus as _mb + +_orig_write = _mb.MotorsBus._write +_orig_sync_write = _mb.MotorsBus._sync_write + + +def _write(self, *args, num_retry=0, **kwargs): + return _orig_write(self, *args, num_retry=max(num_retry, {retries}), **kwargs) + + +def _sync_write(self, *args, num_retry=0, **kwargs): + return _orig_sync_write(self, *args, num_retry=max(num_retry, {retries}), **kwargs) + + +_mb.MotorsBus._write = _write +_mb.MotorsBus._sync_write = _sync_write + +sys.argv[0] = "lerobot_calibrate" +import lerobot.scripts.lerobot_calibrate as _lerobot_calibrate + +_lerobot_calibrate.main() +""" + # lerobot_calibrate takes --robot.* for a robot, --teleop.* for a teleoperator; # the SO-101 leader is registered as a teleoperator, not a second robot. ARMS = { @@ -121,7 +153,7 @@ def check_lerobot_available(): ) -def run_calibration(arm, port, calibration_id): +def run_calibration(arm, port, calibration_id, bus_write_retries=5): spec = ARMS[arm] existing = calibration_json_path(arm, calibration_id) if existing.is_file(): @@ -133,10 +165,16 @@ def run_calibration(arm, port, calibration_id): input( f"Ready to calibrate the {arm} ({calibration_id} on {port})? Press Enter to start." ) + if bus_write_retries > 0: + entry_point = [ + "-c", + BUS_WRITE_RETRY_SHIM.format(retries=bus_write_retries), + ] + else: + entry_point = ["-m", "lerobot.scripts.lerobot_calibrate"] cmd = [ sys.executable, - "-m", - "lerobot.scripts.lerobot_calibrate", + *entry_point, f"--{spec['flag_prefix']}.type={spec['type']}", f"--{spec['flag_prefix']}.port={port}", f"--{spec['flag_prefix']}.id={calibration_id}", @@ -164,6 +202,13 @@ def run_calibration(arm, port, calibration_id): print(f"Wrote {existing}") +def non_negative_int(value): + parsed = int(value) + if parsed < 0: + raise argparse.ArgumentTypeError(f"{value!r} is negative") + return parsed + + def parse_args(argv): parser = argparse.ArgumentParser(description=__doc__) parser.add_argument("arm", choices=sorted(ARMS), help="which bus to calibrate") @@ -176,6 +221,16 @@ def parse_args(argv): default=None, help="LeRobot calibration id (default: so101_)", ) + parser.add_argument( + "--bus-write-retries", + type=non_negative_int, + default=5, + help=( + "floor lerobot_calibrate's bus WRITE retry count at this value " + "(0 disables the wrapping and runs the stock lerobot_calibrate " + "CLI unmodified; default: 5)" + ), + ) return parser.parse_args(argv) @@ -202,7 +257,7 @@ def main(argv=None): spec = ARMS[args.arm] port = args.port or spec["default_port"] calibration_id = args.calibration_id or spec["default_id"] - run_calibration(args.arm, port, calibration_id) + run_calibration(args.arm, port, calibration_id, args.bus_write_retries) if __name__ == "__main__": diff --git a/src/so101_base_config/test/test_calibrate_so101.py b/src/so101_base_config/test/test_calibrate_so101.py index e4cb666df..0e95980cb 100644 --- a/src/so101_base_config/test/test_calibrate_so101.py +++ b/src/so101_base_config/test/test_calibrate_so101.py @@ -61,7 +61,24 @@ def test_parse_args_rejects_an_unknown_arm(): calibrate_so101.parse_args(["gripper"]) -def test_run_calibration_uses_defaults_and_execs_lerobot(monkeypatch, tmp_path): +def test_parse_args_defaults_bus_write_retries_to_five(): + args = calibrate_so101.parse_args(["follower"]) + assert args.bus_write_retries == 5 + + +def test_parse_args_accepts_a_bus_write_retries_override(): + args = calibrate_so101.parse_args(["follower", "--bus-write-retries", "0"]) + assert args.bus_write_retries == 0 + + +def test_parse_args_rejects_a_negative_bus_write_retries(): + with pytest.raises(SystemExit): + calibrate_so101.parse_args(["follower", "--bus-write-retries", "-1"]) + + +def test_run_calibration_uses_defaults_and_execs_lerobot_through_the_retry_shim( + monkeypatch, tmp_path +): subdir = tmp_path / "robots" / "so_follower" monkeypatch.setitem(calibrate_so101.ARMS["follower"], "calibration_subdir", subdir) @@ -78,6 +95,37 @@ def fake_run(cmd, check): calibrate_so101.run_calibration("follower", "/dev/so101_follower", "so101_follower") + assert commands[0] == [ + calibrate_so101.sys.executable, + "-c", + calibrate_so101.BUS_WRITE_RETRY_SHIM.format(retries=5), + "--robot.type=so101_follower", + "--robot.port=/dev/so101_follower", + "--robot.id=so101_follower", + ] + + +def test_run_calibration_with_zero_bus_write_retries_uses_the_stock_invocation( + monkeypatch, tmp_path +): + subdir = tmp_path / "robots" / "so_follower" + monkeypatch.setitem(calibrate_so101.ARMS["follower"], "calibration_subdir", subdir) + + commands = [] + + def fake_run(cmd, check): + assert check + subdir.mkdir(parents=True) + (subdir / "so101_follower.json").write_text("{}") + commands.append(cmd) + + monkeypatch.setattr(calibrate_so101.subprocess, "run", fake_run) + monkeypatch.setattr("builtins.input", lambda *_: "") + + calibrate_so101.run_calibration( + "follower", "/dev/so101_follower", "so101_follower", bus_write_retries=0 + ) + assert commands[0] == [ calibrate_so101.sys.executable, "-m",