fix(cameras): release vanished-camera capture handles; route servo-bus and camera errors to honest hints

makermodslab/teleoperate.py

@@ -653,7 +653,24 @@ def force_disconnect_partial(device, label: str = "device") -> list[str]:
         try:
             cam.disconnect()
         except DeviceNotConnectedError:
-            pass  # never opened (or already released) — nothing to do
+            # Never opened (or already released) — usually nothing to do. But a
+            # camera whose DEVICE VANISHED mid-connect (unplugged between open
+            # and the read thread starting) lands here too: ``is_connected``
+            # goes False on the dead device and ``thread is None``, yet
+            # ``videocapture`` still holds the OS capture session. Left in
+            # place, that stale in-process session poisons every later open of
+            # the replugged camera (degraded fps profile / no frames) until
+            # MakerMods Lab restarts — so release the raw handle directly.
+            videocapture = getattr(cam, "videocapture", None)
+            if videocapture is not None:
+                try:
+                    videocapture.release()
+                    cam.videocapture = None
+                    logger.info(
+                        f"Released {label} camera {name}'s stale capture handle (device vanished mid-connect)"
+                    )
+                except Exception as e:
+                    logger.warning(f"Could not release {label} camera {name}'s capture handle: {e}")
         except Exception as e:
             message = f"Could not release {label} camera {name}: {e}"
             logger.warning(message)

makermodslab/utils/errors.py

@@ -159,6 +159,13 @@ def friendly_hint(error_text: str | None) -> str | None:
             "cable/daisy-chain link is loose. Power-cycle the arm, re-seat the cables, then try "
             "teleoperation before inference."
         )
+    # A camera that won't open at all is unplugged (or the index is stale
+    # after a port change) — distinct from the turbulence family below, where
+    # the device opens but comes up degraded. Kept ahead of the Hub branches:
+    # lerobot raises this as a ConnectionError too, so the arm/camera families
+    # are all resolved before anything can be read as a download problem.
+    if "failed to open" in low and "camera" in low:
+        return "A camera couldn't be opened — it looks unplugged. Check its cable, then try again."
     # Hub model-download failures (snapshot_download, before the arm is ever
     # touched). Keyed on hub-specific tokens so a network/404/disk error while
     # fetching a checkpoint isn't mistaken for an arm-connection problem below.
@@ -183,6 +190,46 @@ def friendly_hint(error_text: str | None) -> str | None:
         return "Couldn't download the model — check your internet connection, then confirm the repo id."
     if "could not connect" in low or "failed to connect" in low or "not connected" in low:
         return "Couldn't connect to the arm — make sure it's plugged in, powered on, and on the right port."
+    # Camera-session turbulence at connect time (N13): the device opened in a
+    # degraded mode or never delivered a warmup frame — either another app
+    # (usually a browser preview) still holds it, or it was just unplugged.
+    # lerobot's message is misleading here, so translate. Must precede the
+    # slow-frames branch: "timed out waiting for frame" would also match
+    # "waiting for frame" there.
+    #
+    # "failed to set fps" is deliberately NOT in this set, even though a
+    # vanished device also reports a nonsense actual_fps. The same marker is
+    # raised by a plain misconfiguration — an operator typing an fps the
+    # device can't do — and nothing in the string separates the two. The
+    # dedicated fps branch below owns it and says "click Auto", which is
+    # actionable for the misconfiguration and harmless for the vanished
+    # device; the reverse (telling someone to check the plug when their
+    # camera simply can't do 60fps) is not. The markers kept here are
+    # unambiguous: they cannot be produced by a settings mistake.
+    #
+    # Marker set kept in sync with record.py's _is_transient_camera_error — a
+    # string that module retries must end up with advice here, or the operator
+    # waits out the retries and is told nothing (credit: #38).
+    #
+    # "read thread is not running" is the marker that actually fires for the
+    # wrong-native-format case (session lands 640x360 instead of 640x480).
+    # lerobot detects that in _postprocess_image, whose only caller is
+    # _read_loop (camera_opencv.py:453) — connect()'s warmup goes through
+    # async_read instead — so the mismatch never propagates synchronously and
+    # the caller sees the dead reader instead. "do not match configured" is
+    # kept only as a forward guard in case upstream ever raises it inline; it
+    # cannot fire on today's connect path, so it must not be the only marker
+    # covering that failure.
+    if (
+        "timed out waiting for frame" in low
+        or "read thread is not running" in low
+        or "do not match configured" in low
+    ):
+        return (
+            "A camera couldn't start in its recording mode — it's either held by another app "
+            "(close other tabs/apps using it, or quit the browser) or was unplugged. "
+            "Check the plug, then try again."
+        )
     if "frame is too old" in low or "no frame" in low or "frame timeout" in low:
         return (
             "A camera can't keep up — frames are arriving too slowly. Lower its resolution/FPS, "

tests/test_rollout.py

@@ -1696,6 +1696,36 @@ def test_friendly_hint_servo_bus_error_is_not_a_download_failure() -> None:
         assert "download" not in hint.lower()
 
 
+def test_friendly_hint_covers_the_marker_the_wrong_format_case_actually_raises() -> None:
+    """The wrong-native-format case (session lands 640x360 instead of the
+    configured 640x480) reaches a synchronous caller as "read thread is not
+    running", NOT as "do not match configured". lerobot detects the mismatch in
+    _postprocess_image, whose only caller is _read_loop (camera_opencv.py:453);
+    connect()'s warmup goes through async_read, so the mismatch text never
+    propagates and the caller sees the dead reader instead. Keying the hint
+    solely on "do not match configured" left the case that actually happens
+    with no advice at all. Credit to #38 for establishing the propagation path.
+    """
+    from makermodslab.utils.errors import friendly_hint
+
+    # The marker that really fires.
+    live = friendly_hint("OpenCVCamera(0) read thread is not running.") or ""
+    assert "held by another app" in live
+
+    # The forward guard still answers if upstream ever raises it inline.
+    guard = (
+        friendly_hint(
+            "OpenCVCamera(0) frame width=640 or height=360 do not match configured width=640 or height=480."
+        )
+        or ""
+    )
+    assert "held by another app" in guard
+
+    # Ordering guard: the slow-frames branch below must not swallow this one.
+    slow = friendly_hint("Camera frame is too old") or ""
+    assert "can't keep up" in slow
+
+
 def test_friendly_hint_still_names_real_download_failures() -> None:
     """The other side of the tightening: a genuine fetch failure keeps its Hub
     hint. Download-step failures reach here with rollout's own

tests/test_teleoperate.py

@@ -959,6 +959,51 @@ def test_force_disconnect_partial_returns_problems_instead_of_none() -> None:
     assert any("TORQUE MAY STILL BE ENABLED" in p and "elbow_flex" in p for p in problems)
 
 
+def test_force_disconnect_partial_releases_vanished_cameras_capture_handle() -> None:
+    """A camera whose DEVICE disappeared mid-connect (unplugged between the
+    cv2 open and the read thread starting) reports not-connected with no
+    thread — but still owns the OS capture session via ``videocapture``.
+    lerobot's disconnect() raises without releasing it (its guard is
+    ``if not self.is_connected and self.thread is None: raise``), and left in
+    place the stale in-process session poisons every later open of the
+    replugged camera until MakerMods Lab restarts. The helper must release the raw
+    handle on exactly this path.
+    """
+    from lerobot.utils.errors import DeviceNotConnectedError
+    from makermodslab.teleoperate import force_disconnect_partial
+
+    class _FakeVideoCapture:
+        def __init__(self) -> None:
+            self.released = False
+
+        def release(self) -> None:
+            self.released = True
+
+    class _VanishedCamera:
+        """lerobot OpenCVCamera's shape at the vanished-device point."""
+
+        def __init__(self) -> None:
+            self.is_connected = False  # isOpened() is False on the dead device
+            self.thread = None  # unplug hit before _start_read_thread
+            self.videocapture = _FakeVideoCapture()
+
+        def disconnect(self) -> None:
+            # Real OpenCVCamera.disconnect() guard, verbatim semantics.
+            if not self.is_connected and self.thread is None:
+                raise DeviceNotConnectedError("OpenCVCamera(0) not connected.")
+
+    bus = _FakeConnectableBus(port="COM_FOLLOWER")
+    cam = _VanishedCamera()
+    capture = cam.videocapture
+    robot = _FakePartialRobot(bus, {"wrist": cam})
+
+    force_disconnect_partial(robot, "robot")
+
+    assert capture.released is True
+    assert cam.videocapture is None
+    assert bus.is_connected is False
+
+
 def test_force_disconnect_partial_is_idempotent_and_handles_bimanual_and_none() -> None:
     from makermodslab.teleoperate import force_disconnect_partial