From 368d2c64d0e5252f05a2c97cd1920d740fa2a807 Mon Sep 17 00:00:00 2001 From: jwgcurrie Date: Sun, 26 Jul 2026 17:56:17 +0200 Subject: [PATCH 1/7] treat a spinning base as still moving in the sim stopMove check --- src/adapters/motion.py | 46 +++++++++++++++++++++-------------- tests/unit/test_adapters.py | 48 +++++++++++++++++++++++++++++++------ 2 files changed, 69 insertions(+), 25 deletions(-) diff --git a/src/adapters/motion.py b/src/adapters/motion.py index 58f1fab..67e200a 100644 --- a/src/adapters/motion.py +++ b/src/adapters/motion.py @@ -1,6 +1,6 @@ -"""ALMotion adapter. Overrides where qibullet's API differs from NAOqi; -stubs for NAOqi methods qibullet doesn't model.""" +"""ALMotion adapter. Overrides where qibullet's API differs from NAOqi; stubs for NAOqi methods qibullet doesn't model.""" +import pybullet from qibullet.base_controller import PepperBaseController from .base import GenericAdapter, stub @@ -8,14 +8,17 @@ class ALMotionAdapter(GenericAdapter): + STOP_TRIES = 5 + STOP_EPS_M_S = 1e-3 + STOP_EPS_RAD_S = 1e-3 + # --- Overrides --- def move(self, x, y, theta): self._pepper.move(float(x), float(y), float(theta)) def moveToward(self, x, y, theta): - # NAOqi: normalized velocity in [-1, 1]. Translate to qibullet's - # absolute m/s and rad/s using the base controller's max constants. + # NAOqi normalizes velocity to [-1, 1]; scale to qibullet's m/s and rad/s. self._pepper.move( float(x) * PepperBaseController.MAX_LINEAR_VELOCITY, float(y) * PepperBaseController.MAX_LINEAR_VELOCITY, @@ -23,24 +26,32 @@ def moveToward(self, x, y, theta): ) def moveTo(self, x, y, theta): - # NAOqi's moveTo is async-by-default; qibullet's is synchronous. - # Force async to match NAOqi semantics. + # NAOqi's moveTo is async by default; qibullet's is synchronous, so force async. self._pepper.moveTo(float(x), float(y), float(theta), _async=True) def stopMove(self): - # qibullet's stopMove only cancels async moveTo. Compose with - # move(0,0,0) to zero the velocity that move() may have set. - self._pepper.stopMove() - self._pepper.move(0, 0, 0) + # qibullet's stopMove only cancels async moveTo and move(0,0,0) races its async process, so retry until the base reads still. + for _ in range(self.STOP_TRIES): + self._pepper.stopMove() + self._pepper.move(0, 0, 0) + linear, angular = pybullet.getBaseVelocity( + self._pepper.robot_model, + physicsClientId=self._pepper.physics_client) + # Rotation shows up only in angular; linear alone misses a spin. + if (max(abs(linear[0]), abs(linear[1])) < self.STOP_EPS_M_S + and abs(angular[2]) < self.STOP_EPS_RAD_S): + return + # A lost stop strands the base walking forever; zero it directly as a backstop. + pybullet.resetBaseVelocity( + self._pepper.robot_model, [0, 0, 0], [0, 0, 0], + physicsClientId=self._pepper.physics_client) def getRobotPosition(self, useSensors=True): - # qibullet's getPosition returns (x, y, theta). NAOqi callers - # expect a list. `useSensors` has no sim equivalent and is ignored. + # qibullet's getPosition returns (x, y, theta); NAOqi callers expect a list, and `useSensors` has no sim equivalent. return list(self._pepper.getPosition()) def getAngles(self, names, useSensors=True): - # qibullet returns a scalar for single-name requests, list otherwise. - # NAOqi's getAngles always returns a list. Wrap scalar. + # qibullet returns a scalar for a single name and NAOqi always returns a list, so wrap it; `useSensors` is ignored. result = self._pepper.getAnglesPosition(names) if isinstance(result, list): return result @@ -50,8 +61,7 @@ def setAngles(self, names, angles, fraction_max_speed): self._pepper.setAngles(names, angles, fraction_max_speed) def angleInterpolation(self, names, angles, durations, isAbsolute=True): - # qibullet has no time-curve interpolation. Run a single setAngles - # at moderate speed; durations and isAbsolute are ignored. + # qibullet has no time-curve interpolation; one setAngles at 0.5 speed, discarding durations and isAbsolute. self._pepper.setAngles(names, angles, 0.5) def wait(self, task_id, timeout_ms=None): @@ -67,7 +77,7 @@ def wakeUp(self): def rest(self): return None - @stub(default=True) + @stub() def robotIsWakeUp(self): return True @@ -87,6 +97,6 @@ def setBreathEnabled(self, *args, **kwargs): def setExternalCollisionProtectionEnabled(self, *args, **kwargs): return None - @stub(default=[]) + @stub() def getRobotConfig(self): return [] diff --git a/tests/unit/test_adapters.py b/tests/unit/test_adapters.py index 856f591..76756aa 100644 --- a/tests/unit/test_adapters.py +++ b/tests/unit/test_adapters.py @@ -72,9 +72,7 @@ def test_generic_post_propagates_unknown_target(): def test_generic_subclass_method_takes_precedence_over_fallthrough(): - """If a subclass defines a method, it must be called instead of falling - through to pepper. This protects against accidentally using - __getattribute__ instead of __getattr__ in the base class.""" + """A subclass method wins over the pepper fall-through (__getattr__, not __getattribute__).""" class MySubclass(GenericAdapter): def myMethod(self): @@ -89,9 +87,7 @@ def myMethod(self): def test_generic_post_forwards_multiple_args_and_kwargs(): - """post must forward arbitrary positional and keyword arguments to the - target. Live callers pass complex payloads (e.g. lookAt takes a point - list, two floats, and a bool).""" + """post forwards arbitrary positional and keyword arguments to the target.""" pepper = MagicMock() adapter = GenericAdapter(pepper, TaskRegistry()) adapter.post("multiArgMethod", 1, "two", 3.0, flag=True, mode="x") @@ -131,8 +127,13 @@ def test_stub_adapter_post_runs_stub_target(): # --- ALMotionAdapter --- +from adapters import motion from adapters.motion import ALMotionAdapter +_STILL = ([0.0, 0.0, 0.0], [0.0, 0.0, 0.0]) +_MOVING = ([0.3, 0.0, 0.0], [0.0, 0.0, 0.0]) +_SPINNING = ([0.0, 0.0, 0.0], [0.0, 0.0, 0.5]) + def _motion_adapter(): return ALMotionAdapter(MagicMock(), TaskRegistry()) @@ -161,13 +162,46 @@ def test_motion_moveto_forces_async(): adapter._pepper.moveTo.assert_called_once_with(1.0, 0.0, 0.0, _async=True) -def test_motion_stopmove_zeros_velocity(): +def test_motion_stopmove_zeros_velocity(monkeypatch): adapter = _motion_adapter() + monkeypatch.setattr(motion.pybullet, "getBaseVelocity", lambda *a, **k: _STILL) adapter.stopMove() adapter._pepper.stopMove.assert_called_once_with() adapter._pepper.move.assert_called_once_with(0, 0, 0) +def test_motion_stopmove_retries_until_base_is_still(monkeypatch): + adapter = _motion_adapter() + reads = iter([_MOVING, _MOVING, _STILL]) + monkeypatch.setattr(motion.pybullet, "getBaseVelocity", lambda *a, **k: next(reads)) + reset = MagicMock() + monkeypatch.setattr(motion.pybullet, "resetBaseVelocity", reset) + adapter.stopMove() + assert adapter._pepper.move.call_count == 3 + reset.assert_not_called() + + +def test_motion_stopmove_does_not_treat_a_spin_as_stopped(monkeypatch): + adapter = _motion_adapter() + reads = iter([_SPINNING, _SPINNING, _STILL]) + monkeypatch.setattr(motion.pybullet, "getBaseVelocity", lambda *a, **k: next(reads)) + reset = MagicMock() + monkeypatch.setattr(motion.pybullet, "resetBaseVelocity", reset) + adapter.stopMove() + assert adapter._pepper.move.call_count == 3 + reset.assert_not_called() + + +def test_motion_stopmove_forces_reset_when_retries_exhausted(monkeypatch): + adapter = _motion_adapter() + monkeypatch.setattr(motion.pybullet, "getBaseVelocity", lambda *a, **k: _MOVING) + reset = MagicMock() + monkeypatch.setattr(motion.pybullet, "resetBaseVelocity", reset) + adapter.stopMove() + assert adapter._pepper.move.call_count == ALMotionAdapter.STOP_TRIES + reset.assert_called_once() + + def test_motion_getrobotposition_returns_list(): adapter = _motion_adapter() adapter._pepper.getPosition.return_value = (1.0, 2.0, 0.5) From 69979bad0036708a6da8efd26d9eff194e59b972 Mon Sep 17 00:00:00 2001 From: jwgcurrie Date: Sun, 26 Jul 2026 17:56:17 +0200 Subject: [PATCH 2/7] drop the decorative stub default and share post dispatch --- src/adapters/__init__.py | 13 +++++----- src/adapters/base.py | 54 ++++++++++++---------------------------- 2 files changed, 22 insertions(+), 45 deletions(-) diff --git a/src/adapters/__init__.py b/src/adapters/__init__.py index e0d95ef..29cdd37 100644 --- a/src/adapters/__init__.py +++ b/src/adapters/__init__.py @@ -1,9 +1,8 @@ -"""Module adapter registry. `build_module_adapters` returns a fresh dict -of `module_name -> adapter` for the dispatcher to use. Construct one set -of adapters per shim process; they hold mutable refs to pepper and the -task registry.""" +"""Module adapter registry. -from .base import GenericAdapter, StubAdapter +`build_module_adapters` returns a fresh `module_name -> adapter` dict; build one set per shim process, since adapters hold mutable refs to pepper and the task registry.""" + +from .base import StubAdapter from .motion import ALMotionAdapter from .memory import ALMemoryAdapter from .posture import ALRobotPostureAdapter @@ -21,9 +20,9 @@ def build_module_adapters(pepper, task_registry): "ALRobotPosture": ALRobotPostureAdapter(pepper, task_registry), "ALLaser": ALLaserAdapter(pepper, task_registry), "ALTextToSpeech": tts, - "ALAnimatedSpeech": tts, # same surface; reuse the same instance + "ALAnimatedSpeech": tts, - # Pure-stub modules (every method returns the default). + # Stub modules (every method returns the default). "ALAnimationPlayer": StubAdapter(default=None, task_registry=task_registry), "ALAutonomousLife": StubAdapter( default=None, diff --git a/src/adapters/base.py b/src/adapters/base.py index e6b7e23..0e2ab38 100644 --- a/src/adapters/base.py +++ b/src/adapters/base.py @@ -1,29 +1,13 @@ """Adapter base classes and the @stub decorator. -GenericAdapter wraps a qibullet `PepperVirtual` and dispatches NAOqi-shaped -method calls. Methods that exist on `pepper` fall through via `__getattr__` -(overrides defined on subclasses take precedence). Unknown methods raise -AttributeError, which the dispatcher surfaces as HTTP 500. - -StubAdapter is for whole-module no-ops (modules where qibullet has no -analogue at all). Every method returns the declared default (or a -per-method override), tagged with `_sim_stub = True` so the dispatcher can -set the `X-Sim-Stub` response header. - -@stub marks individual methods on modeled adapters as sim no-ops without -making the whole adapter a StubAdapter. +GenericAdapter forwards to a qibullet `PepperVirtual` (unknown methods raise AttributeError), StubAdapter no-ops a module, @stub no-ops a method, and every stub carries `_sim_stub = True` for the `X-Sim-Stub` header. """ import functools -def stub(default=None): - """Mark a method as a sim stub. - - Sets `_sim_stub = True` on the wrapper so the dispatcher can flag the - response. The `default` argument is documentation only; pass it to - record the intended return value at the decoration site. - """ +def stub(): + """Mark a method as a sim stub via `_sim_stub = True`; the body is the return value.""" def decorator(func): @functools.wraps(func) def wrapper(*args, **kwargs): @@ -33,7 +17,16 @@ def wrapper(*args, **kwargs): return decorator -class GenericAdapter: +class _PostDispatch: + """NAOqi `post` dispatch shared by both adapter kinds.""" + + def post(self, target_method, *args, **kwargs): + """Run the target synchronously and return its task ID.""" + target = getattr(self, target_method) + return self._tasks.submit_sync(target, args, kwargs) + + +class GenericAdapter(_PostDispatch): """Wraps qibullet's PepperVirtual. Subclasses add overrides and stubs.""" _BLOCKED_NAMES = frozenset({"loadRobot"}) @@ -42,14 +35,8 @@ def __init__(self, pepper, task_registry): self._pepper = pepper self._tasks = task_registry - def post(self, target_method, *args, **kwargs): - """NAOqi `post` dispatch. Runs target synchronously, returns task ID.""" - target = getattr(self, target_method) - return self._tasks.submit_sync(target, args, kwargs) - def __getattr__(self, name): - # __getattr__ runs only when normal lookup fails, so overrides - # defined on subclasses are reached first. + # Runs only when normal lookup fails, so subclass overrides are reached first. if name.startswith("_"): raise AttributeError("{!r} is private".format(name)) if name in self._BLOCKED_NAMES: @@ -59,23 +46,14 @@ def __getattr__(self, name): return getattr(self._pepper, name) -class StubAdapter: - """For modules where every method is a no-op in sim. - - `default` is the return value for any method not in `overrides`. - `overrides` maps method name -> return value for methods that need a - specific non-default response. - """ +class StubAdapter(_PostDispatch): + """No-ops a whole module: every method returns `overrides[name]` if present, else `default`.""" def __init__(self, default=None, overrides=None, task_registry=None): self._default = default self._overrides = overrides or {} self._tasks = task_registry - def post(self, target_method, *args, **kwargs): - target = getattr(self, target_method) - return self._tasks.submit_sync(target, args, kwargs) - def __getattr__(self, name): if name.startswith("_"): raise AttributeError("{!r} is private".format(name)) From 6f4874a76042d0dd40c31ac96917595133379a90 Mon Sep 17 00:00:00 2001 From: jwgcurrie Date: Sun, 26 Jul 2026 17:56:17 +0200 Subject: [PATCH 3/7] split out the joint publisher and stop the simulation on sigterm --- entrypoint.sh | 19 +++++---------- src/joint_publisher.py | 40 ++++++++++++++++++++++++++++++ src/shim_server.py | 55 ++++++++++-------------------------------- 3 files changed, 59 insertions(+), 55 deletions(-) create mode 100644 src/joint_publisher.py diff --git a/entrypoint.sh b/entrypoint.sh index 99f16ad..7d63d8d 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -12,9 +12,7 @@ fi if [ "$MODE" = "true" ]; then echo "[Entrypoint] Starting SIMULATION (qibullet)..." - # Auto-seed the qibullet asset cache if the URDF is missing. The cache - # lives at $HOME/.qibullet and is persisted by a volume mount; the - # installer writes into a version subdirectory (e.g. 1.4.3/). + # Auto-seed the qibullet cache ($HOME/.qibullet, volume-mounted) if pepper.urdf is missing; the installer writes a version subdirectory (e.g. 1.4.3/). if ! ls "$HOME/.qibullet"/*/pepper.urdf >/dev/null 2>&1; then echo "[Entrypoint] qibullet assets missing; seeding via setup_wizard.py..." if ! python3 src/setup_wizard.py; then @@ -24,7 +22,8 @@ if [ "$MODE" = "true" ]; then exit 1 fi fi - python3 src/shim_server.py + # exec so the shim is PID 1 and gets SIGTERM; bash never forwards it to a foreground child. + exec python3 src/shim_server.py else echo "[Entrypoint] Starting PHYSICAL ROBOT Bridge (pynaoqi)..." echo " - Target: $NAOQI_IP:$NAOQI_PORT" @@ -34,10 +33,7 @@ else cat >&2 <&2 < Date: Sun, 26 Jul 2026 17:56:17 +0200 Subject: [PATCH 4/7] assert robot state in the sim integration guards --- tests/guard.py | 114 +++++++++++++++++++++++++++++++++++++++++++ tests/test_motion.py | 73 ++++++++++++++++----------- tests/test_odom.py | 106 +++++++++++++++++++++++++++------------- tests/test_sonar.py | 61 ++++++++++++++--------- 4 files changed, 267 insertions(+), 87 deletions(-) create mode 100644 tests/guard.py diff --git a/tests/guard.py b/tests/guard.py new file mode 100644 index 0000000..3247426 --- /dev/null +++ b/tests/guard.py @@ -0,0 +1,114 @@ +"""Shared helpers for the shim integration guards. + +Each guard runs as a script (exit 1 on failure) and under pytest (skipped when no shim answers). +""" +import math +import os +import sys +import time + +sys.path.append(os.path.join(os.path.dirname(__file__), os.pardir, "py3-naoqi-bridge")) + +from naoqi_proxy import NaoqiClient # noqa: E402 + + +def connect(): + return NaoqiClient() + + +def shim_reachable(): + """True if a shim answers on localhost:5000.""" + try: + NaoqiClient().ALMotion.getRobotPosition(True) + return True + except Exception: + return False + + +def wrap_angle(radians): + """Fold an angle difference into (-pi, pi]; getRobotPosition wraps theta.""" + return math.atan2(math.sin(radians), math.cos(radians)) + + +def settled(client, seconds=1.0): + """Pose sampled twice `seconds` apart, returned as (before, after).""" + before = client.ALMotion.getRobotPosition(True) + time.sleep(seconds) + return before, client.ALMotion.getRobotPosition(True) + + +def wait_until_still(client, timeout=45.0, tol=0.002, hold=0.6): + """Poll until the base pose holds steady for `hold` seconds; False on timeout. + + `timeout` is a hang guard, not an expected duration: a 0.8 rad turn takes ~4.8s idle, ~14s loaded. + """ + deadline = time.time() + timeout + last = client.ALMotion.getRobotPosition(True) + still_since = None + while time.time() < deadline: + time.sleep(0.2) + now = client.ALMotion.getRobotPosition(True) + moved = math.hypot(now[0] - last[0], now[1] - last[1]) + spun = abs(wrap_angle(now[2] - last[2])) + if moved < tol and spun < tol: + if still_since is None: + still_since = time.time() + elif time.time() - still_since >= hold: + return True + else: + still_since = None + last = now + return False + + +def spin_until(client, target_rad, eps=0.0001, stall_polls=25, poll_s=0.1): + """Rotate until |dtheta| reaches target_rad; returns the angle turned. + + Gives up on stall rather than elapsed time, so the verdict does not depend on host speed. + """ + theta = lambda: client.ALMotion.getRobotPosition(True)[2] + start = last = theta() + stalled = 0 + while stalled < stall_polls: + time.sleep(poll_s) + now = theta() + if abs(wrap_angle(now - start)) >= target_rad: + break + stalled = stalled + 1 if abs(wrap_angle(now - last)) < eps else 0 + last = now + return abs(wrap_angle(theta() - start)) + + +class Guard: + """Accumulates named checks and fails once at the end with the full list.""" + + def __init__(self, name): + self.name = name + self.failures = [] + + def check(self, label, ok, detail=""): + print(f"[{'PASS' if ok else 'FAIL'}] {label}" + (f" ({detail})" if detail else "")) + if not ok: + self.failures.append(label) + + def stationary(self, client, label="base is stationary", tol=0.005, seconds=1.0): + """Assert the base is not drifting; a lost stop strands the sim walking away.""" + a, b = settled(client, seconds) + moved = math.hypot(b[0] - a[0], b[1] - a[1]) + spun = abs(wrap_angle(b[2] - a[2])) + self.check(label, moved <= tol and spun <= tol, f"d={moved:.4f}m dtheta={spun:.4f}rad") + + def finish(self): + if self.failures: + raise AssertionError( + f"{self.name}: {len(self.failures)} check(s) failed: {self.failures}") + print(f"\n--- {self.name}: all checks passed ---") + + +def main(fn): + """Run a guard function as a script, exiting 1 on failure.""" + try: + fn() + except AssertionError as exc: + print(f"\n--- FAILURE ---\n{exc}") + sys.exit(1) diff --git a/tests/test_motion.py b/tests/test_motion.py index 33fdc4c..eeca50c 100644 --- a/tests/test_motion.py +++ b/tests/test_motion.py @@ -1,36 +1,51 @@ -import sys -import os +"""Integration guard: joint command/read-back, posture, and that stopMove halts the base. -# Add the bridge folder to path so we can import modules from it -# We are in tests/, so bridge is in ../py3-naoqi-bridge -bridge_path = os.path.join(os.path.dirname(__file__), "..", "py3-naoqi-bridge") -sys.path.append(bridge_path) - -from naoqi_proxy import NaoqiClient -import math +Requires a running shim (./run.sh). Run as a script or under pytest. +""" import time +from guard import Guard, connect, shim_reachable, main + +try: + import pytest + pytestmark = pytest.mark.skipif(not shim_reachable(), reason="requires a running shim") +except ImportError: + pass + +JOINT_TOL_RAD = 0.02 + + def test_motion(): - print("Connecting to PepperBox Shim...") - client = NaoqiClient() # defaults to localhost:5000 - - print("Commanding: Rotate Left (Z-axis)...") - # ALMotion.move(x, y, theta) -> Velocity control - # Spin at 0.5 rad/s + g = Guard("motion") + client = connect() + + # Commanded joint angles must come back from the read-back path. + client.ALMotion.setAngles(["HeadYaw"], [0.5], 0.3) + time.sleep(2.0) + read = client.ALMotion.getAngles(["HeadYaw"], True) + g.check("getAngles returns a list", isinstance(read, list) and len(read) == 1, f"{read}") + if isinstance(read, list) and read: + g.check("HeadYaw tracks command", abs(read[0] - 0.5) <= JOINT_TOL_RAD, + f"cmd=0.500 read={read[0]:.3f}") + + # qibullet returns a scalar for a single joint; the adapter must still hand back a list. + single = client.ALMotion.getAngles("HeadYaw", True) + g.check("getAngles wraps a scalar into a list", isinstance(single, list), f"{single}") + + client.ALMotion.setAngles(["HeadYaw"], [0.0], 0.3) + time.sleep(1.5) + + g.check("goToPosture Stand succeeds", + client.ALRobotPosture.goToPosture("Stand", 0.5) is True) + + # A rotation must actually stop; a lost stop walks the sim off the world. client.ALMotion.move(0.0, 0.0, 0.5) - - print("Wait 3 seconds...") - time.sleep(3) - - print("Commanding: Stop") - client.ALMotion.move(0.0, 0.0, 0.0) - - print("Commanding: Move Head Pitch...") - # Only if your shim implements 'setAngles' or similar for joints - # client.ALMotion.setStiffnesses("Head", 1.0) - # client.ALMotion.setAngles("HeadPitch", 0.5, 0.1) - - print("Motion Test Complete.") + time.sleep(2.0) + client.ALMotion.stopMove() + g.stationary(client, "base halts after stopMove") + + g.finish() + if __name__ == "__main__": - test_motion() + main(test_motion) diff --git a/tests/test_odom.py b/tests/test_odom.py index 2187125..a602d30 100644 --- a/tests/test_odom.py +++ b/tests/test_odom.py @@ -1,37 +1,75 @@ -import sys -import os -import time - -# Add the bridge folder to path so we can import modules from it -bridge_path = os.path.join(os.path.dirname(__file__), "..", "py3-naoqi-bridge") -sys.path.append(bridge_path) - -from naoqi_proxy import NaoqiClient - -def test_odom_vis(): - print("Connecting to PepperBox Shim (Odom & Vis Test)...") - client = NaoqiClient() - - print("\n1. Enabling Laser Visualization (Check Simulator Window!)...") - # Using our custom ALLaser module +"""Integration guard: odometry tracks commanded rotation and translation, and motion stops. + +Requires a running shim (./run.sh). Run as a script or under pytest. +""" +import math + +from guard import (Guard, connect, main, shim_reachable, spin_until, wait_until_still, + wrap_angle) + +try: + import pytest + pytestmark = pytest.mark.skipif(not shim_reachable(), reason="requires a running shim") +except ImportError: + pass + +SPIN_RAD_S = 0.5 +MIN_SPIN_RAD = 0.05 +TURN_RAD = 0.8 +TURN_TOL_RAD = 0.08 +STEP_M = 0.4 +STEP_TOL_M = 0.05 + + +def test_odom(): + g = Guard("odom") + client = connect() client.ALLaser.show(True) - - print("2. Monitoring Odometry while rotating...") - # Start rotating - client.ALMotion.move(0.0, 0.0, 0.5) - - try: - for i in range(10): - # getRobotPosition(useSensors) -> [x, y, theta] - pos = client.ALMotion.getRobotPosition(True) - print(f"Odom: X={pos[0]:.3f}, Y={pos[1]:.3f}, Theta={pos[2]:.3f}") - time.sleep(0.5) - - finally: - print("Stopping...") - client.ALMotion.stopMove() - # Optionally turn off lasers - # client.ALLaser.show(False) + + # Hold a velocity until the base actually turns; the sim steps as its thread is scheduled, so a timed command covers a host-dependent angle. + client.ALMotion.move(0.0, 0.0, SPIN_RAD_S) + spun = spin_until(client, MIN_SPIN_RAD) + client.ALMotion.stopMove() + g.check("velocity command rotates the base", spun >= MIN_SPIN_RAD, f"dtheta={spun:.3f}rad") + g.stationary(client, "base halts after a rotation") + + # Unwind so repeated runs do not leave the base at a random heading. + client.ALMotion.move(0.0, 0.0, -SPIN_RAD_S) + spin_until(client, spun) + client.ALMotion.stopMove() + + # moveTo is robot-frame, so measuring mid-turn traces a curve and under-reports; assert the precondition. + g.check("base still before closed-loop moves", wait_until_still(client)) + + # moveTo is a displacement command, so the angle achieved does not depend on sim speed. + before_theta = client.ALMotion.getRobotPosition(True)[2] + client.ALMotion.moveTo(0.0, 0.0, TURN_RAD) + g.check("rotation settles", wait_until_still(client)) + turned = wrap_angle(client.ALMotion.getRobotPosition(True)[2] - before_theta) + g.check("moveTo turns by the commanded angle", abs(turned - TURN_RAD) <= TURN_TOL_RAD, + f"turned={turned:.3f}rad commanded={TURN_RAD}rad") + client.ALMotion.moveTo(0.0, 0.0, -turned) + wait_until_still(client) + + # Translation must register a displacement of roughly the commanded distance. + before = client.ALMotion.getRobotPosition(True) + client.ALMotion.moveTo(STEP_M, 0.0, 0.0) + g.check("translation settles", wait_until_still(client)) + after = client.ALMotion.getRobotPosition(True) + moved = math.hypot(after[0] - before[0], after[1] - before[1]) + g.check("moveTo travels the commanded distance", abs(moved - STEP_M) <= STEP_TOL_M, + f"moved={moved:.3f}m commanded={STEP_M}m") + g.check("translation holds heading", abs(wrap_angle(after[2] - before[2])) < 0.2, + f"dtheta={wrap_angle(after[2] - before[2]):.3f}rad") + + # Step back by the distance actually measured, then confirm nothing is still driving. + client.ALMotion.moveTo(-moved, 0.0, 0.0) + wait_until_still(client) + client.ALMotion.stopMove() + g.stationary(client, "base left stationary") + + g.finish() + if __name__ == "__main__": - test_odom_vis() + main(test_odom) diff --git a/tests/test_sonar.py b/tests/test_sonar.py index d1e546c..dea4384 100644 --- a/tests/test_sonar.py +++ b/tests/test_sonar.py @@ -1,30 +1,43 @@ -import sys -import os -import time +"""Integration guard: sonar and laser readings have the right shape and a live range. -# Add the bridge folder to path so we can import modules from it -bridge_path = os.path.join(os.path.dirname(__file__), "..", "py3-naoqi-bridge") -sys.path.append(bridge_path) +Requires a running shim (./run.sh). Run as a script or under pytest. +""" +from guard import Guard, connect, shim_reachable, main + +try: + import pytest + pytestmark = pytest.mark.skipif(not shim_reachable(), reason="requires a running shim") +except ImportError: + pass + +MAX_RANGE_M = 3.0 +LASER_RAYS = 15 + +FRONT = "Device/SubDeviceList/Platform/Front/Sonar/Sensor/Value" +BACK = "Device/SubDeviceList/Platform/Back/Sonar/Sensor/Value" +LASER_FRONT = "Device/SubDeviceList/Platform/Laser/Front/Sensor/Value" -from naoqi_proxy import NaoqiClient def test_sonar(): - print("Connecting to PepperBox Shim (Sonar Test)...") - client = NaoqiClient() - - print("Reading Sonar Values...") - # Front Sonar - front = client.ALMemory.getData("Device/SubDeviceList/Platform/Front/Sonar/Sensor/Value") - print(f"Front Sonar: {front} meters") - - # Back Sonar - back = client.ALMemory.getData("Device/SubDeviceList/Platform/Back/Sonar/Sensor/Value") - print(f"Back Sonar: {back} meters") - - if front is not None and back is not None: - print("\nSUCCESS: Sonar values received (simulated via Laser).") - else: - print("\nFAILURE: One or more sonar values were None.") + g = Guard("sonar") + client = connect() + client.ALLaser.show(True) + + for label, key in (("front", FRONT), ("back", BACK)): + value = client.ALMemory.getData(key) + # 0.0 is what an unscanned laser reports, indistinguishable from an obstacle at contact. + g.check(f"{label} sonar is a live range", + isinstance(value, float) and 0.0 < value <= MAX_RANGE_M, f"{value}") + + rays = client.ALMemory.getData(LASER_FRONT) + g.check("front laser returns a list", isinstance(rays, list), f"{type(rays).__name__}") + if isinstance(rays, list): + g.check(f"front laser returns {LASER_RAYS} rays", len(rays) == LASER_RAYS, f"{len(rays)}") + g.check("laser rays are floats in range", + all(isinstance(r, float) and 0.0 <= r <= MAX_RANGE_M for r in rays)) + + g.finish() + if __name__ == "__main__": - test_sonar() + main(test_sonar) From 4d3e30eed593028cd9d149eff618446a1a570e75 Mon Sep 17 00:00:00 2001 From: jwgcurrie Date: Sun, 26 Jul 2026 17:56:17 +0200 Subject: [PATCH 5/7] make the python 2 bridge testable and add shim coverage --- py3-naoqi-bridge/README.md | 17 ++- py3-naoqi-bridge/tests/py2/__init__.py | 0 .../tests/{ => py2}/test_audio_publisher.py | 0 .../tests/py2/test_shim_server.py | 133 ++++++++++++++++++ tests/run-py2-in-docker.sh | 29 ++++ 5 files changed, 172 insertions(+), 7 deletions(-) create mode 100644 py3-naoqi-bridge/tests/py2/__init__.py rename py3-naoqi-bridge/tests/{ => py2}/test_audio_publisher.py (100%) create mode 100644 py3-naoqi-bridge/tests/py2/test_shim_server.py create mode 100755 tests/run-py2-in-docker.sh diff --git a/py3-naoqi-bridge/README.md b/py3-naoqi-bridge/README.md index ced4734..2990c92 100644 --- a/py3-naoqi-bridge/README.md +++ b/py3-naoqi-bridge/README.md @@ -1,6 +1,6 @@ # Python 3 to NAOqi Bridge -This directory contains the Python 2.7 "shim" server and the Python 3 "proxy" client. When put together they form a bridge to allow Python 3 applications to communicate with the `naoqi` robot SDK. +This directory contains the Python 2.7 shim server and the Python 3 proxy client. When put together they form a bridge to allow Python 3 applications to communicate with the `naoqi` robot SDK. ## Purpose @@ -10,12 +10,12 @@ The `pynaoqi` SDK is only compatible with Python 2.7. The Python 2.7 shim server * **`shim_server.py`**: The Python 2.7 Flask server that exposes the NAOqi API via HTTP. * **`naoqi_proxy.py`**: The Python 3 client library that provides an interface to interact with the shim server. Exposes the optional `warn_on_stubs` flag and `SimStubWarning` class for sim-mode introspection. -* **`examples/`**: Directory containing example scripts demonstrating the usage of the `NaoqiClient` (e.g., `basic_usage_example.py`, `helloworld_in_python3.py`). -* **`tests/`**: Unit and integration tests. `test_audio_publisher.py` covers the ZMQ audio publisher; `test_motion.py` is a scripted integration test against a running shim; `test_sim_stub_warning.py` covers the client-side opt-in warning behavior with mocked HTTP responses. +* **`tests/`**: Python 3 tests. `test_motion.py` is a scripted integration test against a running shim; `test_sim_stub_warning.py` covers the client-side opt-in warning behavior with mocked HTTP responses. +* **`tests/py2/`**: Python 2.7 tests for the shim side, which import the bridge modules with the `naoqi` SDK stubbed. `test_shim_server.py` covers the REST contract and the Py2 unicode marshalling; `test_audio_publisher.py` covers the ZMQ audio publisher. Kept separate because the two halves need different interpreters. ## Using the NaoqiClient -The shim server is launched by the project-level `./run.sh` — see the top-level README for setup and configuration (`NAOQI_IP`, `NAOQI_PORT`, `robot.env`, etc.). Once the shim is running on port 5000, Python 3 code uses the `NaoqiClient` to interface. +The shim server is launched by the project-level `./run.sh`, see the top-level README for setup and configuration (`NAOQI_IP`, `NAOQI_PORT`, `robot.env`, etc.). Once the shim is running on port 5000, Python 3 code uses the `NaoqiClient` to interface. ### Initialisation @@ -49,13 +49,16 @@ Errors from the shim or the underlying NAOqi instance surface as `NaoqiProxyErro ## Testing the Bridge -Unit tests for the client library and the audio publisher run inside the pepper-box container via the project-wide wrapper: +Both halves run inside the pepper-box container, each via its own wrapper because the shim is Python 2.7 and the client library is Python 3. ```bash -./tests/run-in-docker.sh py3-naoqi-bridge/tests/test_sim_stub_warning.py -v -./tests/run-in-docker.sh py3-naoqi-bridge/tests/test_audio_publisher.py -v +./tests/run-in-docker.sh py3-naoqi-bridge/tests/test_sim_stub_warning.py -v # Python 3 +./tests/run-py2-in-docker.sh # Python 2, all of tests/py2 +./tests/run-py2-in-docker.sh tests/py2/test_shim_server.py -v # a single Py2 file ``` +The Py2 tests stub the `naoqi` SDK, so they need no robot. What still needs hardware is NAOqi behaviour itself: that `ALProxy` connects, that `ALModule.autoBind` registers `processRemote`, and the broker reverse path. + The scripted integration test `tests/test_motion.py` requires a running shim and a real or simulated robot. For the wider sim-side test suite (adapters, dispatcher, Flask routes) see the top-level `tests/unit/` directory and the project root README. diff --git a/py3-naoqi-bridge/tests/py2/__init__.py b/py3-naoqi-bridge/tests/py2/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/py3-naoqi-bridge/tests/test_audio_publisher.py b/py3-naoqi-bridge/tests/py2/test_audio_publisher.py similarity index 100% rename from py3-naoqi-bridge/tests/test_audio_publisher.py rename to py3-naoqi-bridge/tests/py2/test_audio_publisher.py diff --git a/py3-naoqi-bridge/tests/py2/test_shim_server.py b/py3-naoqi-bridge/tests/py2/test_shim_server.py new file mode 100644 index 0000000..df8af85 --- /dev/null +++ b/py3-naoqi-bridge/tests/py2/test_shim_server.py @@ -0,0 +1,133 @@ +"""Unit tests for the physical-robot shim, with the naoqi SDK stubbed. + +Covers the REST contract and the Py2 unicode marshalling; connecting to a real broker still needs hardware. +""" +import sys +import types +import unittest + +# Stub the proprietary SDK before importing the module under test. +if "naoqi" not in sys.modules: + _naoqi = types.ModuleType("naoqi") + _naoqi.ALProxy = lambda *a, **k: None + sys.modules["naoqi"] = _naoqi + +import shim_server # noqa: E402 + + +class _FakePost(object): + """ALProxy.post is a distinct object whose methods dispatch asynchronously.""" + + def __init__(self, parent): + self._parent = parent + + def echo(self, *args): + self._parent.calls.append(("post.echo", args)) + return 99 # NAOqi returns a task id, not the result + + +class _FakeProxy(object): + """Stands in for ALProxy: records calls and echoes arguments back.""" + + def __init__(self, name="ALMotion"): + self.name = name + self.calls = [] + self.post = _FakePost(self) + + def echo(self, *args): + self.calls.append(("echo", args)) + return list(args) + + def boom(self, *args): + raise RuntimeError("naoqi exploded") + + +class ShimTestCase(unittest.TestCase): + def setUp(self): + shim_server.PROXY_CACHE.clear() + self.proxy = _FakeProxy() + shim_server.PROXY_CACHE["ALMotion"] = self.proxy + shim_server.app.config["TESTING"] = True + self.client = shim_server.app.test_client() + + def post(self, payload): + import json + return self.client.post("/api/call", data=json.dumps(payload), + content_type="application/json") + + +class TestEncoding(unittest.TestCase): + def test_unicode_becomes_utf8_str(self): + out = shim_server._deep_encode_to_str(u"Bonjour") + self.assertIsInstance(out, str) + self.assertEqual(out, "Bonjour") + + def test_encoding_recurses_into_containers(self): + out = shim_server._deep_encode_to_str([u"a", (u"b",), {u"k": u"v"}]) + self.assertEqual(out, ["a", ("b",), {"k": "v"}]) + self.assertIsInstance(out[2].keys()[0], str) + + def test_non_strings_pass_through(self): + self.assertEqual(shim_server._deep_encode_to_str([1, 2.5, True, None]), + [1, 2.5, True, None]) + + def test_decoding_returns_unicode(self): + out = shim_server._deep_decode_to_unicode(["a", {"k": "v"}]) + self.assertIsInstance(out[0], unicode) # noqa: F821 + self.assertIsInstance(out[1].keys()[0], unicode) # noqa: F821 + + def test_undecodable_bytes_pass_through(self): + raw = b"\xff\xfe" + self.assertEqual(shim_server._deep_decode_to_unicode(raw), raw) + + +class TestProxyCache(ShimTestCase): + def test_proxy_is_reused_across_calls(self): + first = shim_server.get_proxy("ALMotion") + second = shim_server.get_proxy("ALMotion") + self.assertIs(first, second) + + +class TestRoute(ShimTestCase): + def test_missing_module_is_rejected(self): + self.assertEqual(self.post({"method": "echo"}).status_code, 400) + + def test_missing_method_is_rejected(self): + self.assertEqual(self.post({"module": "ALMotion"}).status_code, 400) + + def test_empty_body_is_rejected(self): + response = self.client.post("/api/call", data="", content_type="application/json") + self.assertEqual(response.status_code, 400) + + def test_call_returns_result(self): + response = self.post({"module": "ALMotion", "method": "echo", "args": [1, 2]}) + self.assertEqual(response.status_code, 200) + import json + self.assertEqual(json.loads(response.data)["result"], [1, 2]) + + def test_args_default_to_empty(self): + response = self.post({"module": "ALMotion", "method": "echo"}) + self.assertEqual(response.status_code, 200) + self.assertEqual(self.proxy.calls[-1], ("echo", ())) + + def test_raising_method_returns_500_with_message(self): + response = self.post({"module": "ALMotion", "method": "boom"}) + self.assertEqual(response.status_code, 500) + import json + self.assertIn("naoqi exploded", json.loads(response.data)["error"]) + + def test_post_dispatches_through_the_post_object(self): + response = self.post({"module": "ALMotion", "method": "post", "args": ["echo", 7]}) + self.assertEqual(response.status_code, 200) + # Must go via proxy.post.echo, not proxy.echo, or the call is synchronous. + self.assertEqual(self.proxy.calls[-1], ("post.echo", (7,))) + import json + self.assertEqual(json.loads(response.data)["result"], 99) + + def test_post_without_a_target_is_an_error(self): + response = self.post({"module": "ALMotion", "method": "post", "args": []}) + self.assertEqual(response.status_code, 500) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/run-py2-in-docker.sh b/tests/run-py2-in-docker.sh new file mode 100755 index 0000000..caf40eb --- /dev/null +++ b/tests/run-py2-in-docker.sh @@ -0,0 +1,29 @@ +#!/bin/bash +# Run pytest under Python 2.7 inside the pepper-box image, for the physical-robot bridge. +# +# The bridge imports the proprietary `naoqi` SDK; these tests stub it, so no robot is needed. +# Still needs hardware: that ALProxy connects, that ALModule.autoBind registers processRemote, and the broker reverse path. +# +# Usage: ./tests/run-py2-in-docker.sh [pytest-args...] +# ./tests/run-py2-in-docker.sh tests/py2/test_shim_server.py -v +# +# Python 2 tests live in py3-naoqi-bridge/tests/py2/; the rest of that directory is Python 3. + +set -e + +SCRIPT_DIR="$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )" +REPO_ROOT="$(dirname "$SCRIPT_DIR")" + +# Default to the py2 suite unless the caller named a path; a bare flag must not displace it. +ARGS=("$@") +for arg in "$@"; do + case "$arg" in -*) ;; *) HAS_PATH=1 ;; esac +done +[ -n "$HAS_PATH" ] || ARGS+=("tests/py2") + +exec docker run --rm \ + -v "$REPO_ROOT:/repo" \ + -w /repo/py3-naoqi-bridge \ + ghcr.io/action-prediction-lab/pepper-box:latest \ + bash -c 'python2 -m pip install --quiet --user "pytest<5" >/dev/null 2>&1 + PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=. python2 -m pytest "$@"' -- "${ARGS[@]}" From 0915534223291c3f25d8c7066e31b685902b47de Mon Sep 17 00:00:00 2001 From: jwgcurrie Date: Sun, 26 Jul 2026 17:56:17 +0200 Subject: [PATCH 6/7] remove the superseded proprioception service, broken examples, and dead imports --- README.md | 2 +- .../examples/basic_usage_example.py | 24 ------ .../examples/helloworld_in_python3.py | 10 --- py3-naoqi-bridge/proprioception_service.py | 78 ------------------- py3-naoqi-bridge/shim_server.py | 13 +--- py3-naoqi-bridge/video_streamer.py | 57 ++------------ tests/unit/test_post_tasks.py | 1 - tests/unit/test_shim_server.py | 4 +- 8 files changed, 13 insertions(+), 176 deletions(-) delete mode 100644 py3-naoqi-bridge/examples/basic_usage_example.py delete mode 100644 py3-naoqi-bridge/examples/helloworld_in_python3.py delete mode 100644 py3-naoqi-bridge/proprioception_service.py diff --git a/README.md b/README.md index 471d340..d6e85de 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # PepperBox -PepperBox is a containerised gateway to the Pepper robot, wrapping legacy Python 2 NAOqi dependencies behind a modern HTTP and ZMQ API so researchers can deploy reproducibly and build applications for Pepper in modern Python 3. +PepperBox is a containerised gateway to the Pepper robot, wrapping legacy Python 2 NAOqi dependencies behind a modern HTTP and ZMQ API so developers and researchers can deploy reproducibly and build applications for Pepper in modern Python 3. PepperBox bundles the `qibullet` simulator, so you can develop without a physical Pepper and run automated experiments headlessly. diff --git a/py3-naoqi-bridge/examples/basic_usage_example.py b/py3-naoqi-bridge/examples/basic_usage_example.py deleted file mode 100644 index 228ee28..0000000 --- a/py3-naoqi-bridge/examples/basic_usage_example.py +++ /dev/null @@ -1,24 +0,0 @@ -from ..naoqi_proxy import NaoqiClient, NaoqiProxyError - -if __name__ == "__main__": - print("Testing the bridge by making the robot say 'Hello from Python 3' using the NaoqiClient proxy.") - - client = NaoqiClient() - - try: - # Example: Make the robot say something - client.ALTextToSpeech.say("Hello from Python 3") - print("Request sent successfully.") - - # Example: Get robot configuration - robot_config = client.ALMotion.getRobotConfig() - print(f"Robot configuration: {robot_config}") - - # Example: Insert data into ALMemory - client.ALMemory.insertData("myKey", "myValue") - print("ALMemory.insertData successful.") - - except NaoqiProxyError as e: - print(f"NAOqi Proxy Error: {e}") - except Exception as e: - print(f"An unexpected error occurred: {e}") \ No newline at end of file diff --git a/py3-naoqi-bridge/examples/helloworld_in_python3.py b/py3-naoqi-bridge/examples/helloworld_in_python3.py deleted file mode 100644 index af1cc9d..0000000 --- a/py3-naoqi-bridge/examples/helloworld_in_python3.py +++ /dev/null @@ -1,10 +0,0 @@ -from ..naoqi_proxy import NaoqiClient, NaoqiProxyError - -try: - client = NaoqiClient() - client.ALTextToSpeech.say("Hello, world!") - print("Successfully made the robot say 'Hello, world!' via Python 3 proxy.") -except NaoqiProxyError as e: - print(f"NAOqi Proxy Error: {e}") -except Exception as e: - print(f"An unexpected error occurred: {e}") \ No newline at end of file diff --git a/py3-naoqi-bridge/proprioception_service.py b/py3-naoqi-bridge/proprioception_service.py deleted file mode 100644 index 70b2eff..0000000 --- a/py3-naoqi-bridge/proprioception_service.py +++ /dev/null @@ -1,78 +0,0 @@ -import os -import sys -import time -import zmq -import struct -from naoqi import ALProxy - -# --- Configuration Loader --- -import argparse - -RATE = 50 # 50Hz - -def get_config(): - parser = argparse.ArgumentParser(description="Pepper Proprioception Service") - parser.add_argument("--ip", type=str, default=os.getenv("NAOQI_IP"), help="Robot IP (or set NAOQI_IP env)") - parser.add_argument("--port", type=int, default=int(os.getenv("NAOQI_PORT", 9559)), help="Robot Port (or set NAOQI_PORT env)") - parser.add_argument("--zmq_port", type=int, default=5560, help="ZMQ PUB Port") - args = parser.parse_args() - - if not args.ip: - raise ValueError("Robot IP must be provided via --ip or NAOQI_IP environment variable.") - - return args - -def main(): - args = get_config() - print("Starting Proprioception Service on {}:{} -> ZMQ:{}...".format(args.ip, args.port, args.zmq_port)) - - # Connect to NaoQi - try: - motion_proxy = ALProxy("ALMotion", args.ip, args.port) - except Exception as e: - print("Error connecting to ALMotion: {}".format(e)) - sys.exit(1) - - # Setup ZMQ - context = zmq.Context() - socket = context.socket(zmq.PUB) - socket.bind("tcp://*:{}".format(args.zmq_port)) - - print("Publishing joint states on port TCP/{} at {} Hz...".format(args.zmq_port, RATE)) - - try: - while True: - start_time = time.time() - - # Get Angles - # useSensors = True ensures we get encoder values, not command values - try: - # 20ms blocking call roughly? No, ALMotion is fast. - names = ["HeadYaw", "HeadPitch"] - angles = motion_proxy.getAngles(names, True) - - if angles and len(angles) == 2: - current_time = time.time() - # Format: Timestamp (double), HeadYaw (float), HeadPitch (float) - # using 'dff' struct format - # Message matches: [topic, binary_data] - # Topic: "joints" - data = struct.pack('dff', current_time, angles[0], angles[1]) - socket.send_multipart(["joints", data]) - - except Exception as e: - print("Error reading joints: {}".format(e)) - - # Sleep to maintain Rate - elapsed = time.time() - start_time - if elapsed < 1.0 / RATE: - time.sleep((1.0 / RATE) - elapsed) - - except KeyboardInterrupt: - print("Interrupted") - finally: - socket.close() - context.term() - -if __name__ == "__main__": - main() diff --git a/py3-naoqi-bridge/shim_server.py b/py3-naoqi-bridge/shim_server.py index f080f17..fc0b3cc 100755 --- a/py3-naoqi-bridge/shim_server.py +++ b/py3-naoqi-bridge/shim_server.py @@ -5,11 +5,9 @@ from flask import Flask, request, jsonify from naoqi import ALProxy -# --- Thread-Safety Fix --- -# A lock to make the global PROXY_CACHE thread-safe +# Guards PROXY_CACHE; Flask serves requests on multiple threads. PROXY_LOCK = threading.Lock() -# --- Configuration --- script_dir = os.path.dirname(os.path.abspath(__file__)) env_file_path = os.path.join(script_dir, 'robot.env') try: @@ -17,7 +15,7 @@ for line in f: if line.strip() and not line.startswith('#'): key, value = line.strip().split('=', 1) - # Only load from file if NOT already set in environment (Docker priority) + # Environment takes precedence over robot.env (Docker priority). if key not in os.environ: os.environ[key] = value print("Loaded configuration from {}".format(env_file_path)) @@ -29,17 +27,14 @@ ROBOT_IP = os.getenv("NAOQI_IP", "127.0.0.1") ROBOT_PORT = int(os.getenv("NAOQI_PORT", 9559)) -# --- PERFORMANCE FIX: Global Proxy Cache --- -# This prevents re-connecting 1000 times per second +# One cached ALProxy per module; reconnecting per request is far too slow. PROXY_CACHE = {} def get_proxy(module_name): with PROXY_LOCK: - global PROXY_CACHE if module_name in PROXY_CACHE: return PROXY_CACHE[module_name] - # Create new connection only if needed py2_module_name = module_name.encode('utf-8') py2_robot_ip = str(ROBOT_IP) @@ -80,10 +75,8 @@ def call_naoqi_method(): kwargs = data.get("kwargs", {}) try: - # 1. Get Cached Proxy (Fast!) proxy = get_proxy(module_name) - # 2. Handle 'post' or standard calls if method_name == "post": if not args: raise ValueError("post requires method name as first arg") diff --git a/py3-naoqi-bridge/video_streamer.py b/py3-naoqi-bridge/video_streamer.py index c330f3c..badab47 100644 --- a/py3-naoqi-bridge/video_streamer.py +++ b/py3-naoqi-bridge/video_streamer.py @@ -2,13 +2,10 @@ import sys import time import zmq -import zmq import struct import vision_definitions -import struct from naoqi import ALProxy -# --- Configuration Loader --- script_dir = os.path.dirname(os.path.abspath(__file__)) env_file_path = os.path.join(script_dir, 'robot.env') try: @@ -29,27 +26,21 @@ def main(): print("Starting Video Streamer...") - # Connect to NaoQi try: video_proxy = ALProxy("ALVideoDevice", ROBOT_IP, ROBOT_PORT) + # motion_proxy is never used again; it exits early if ALMotion is unreachable. motion_proxy = ALProxy("ALMotion", ROBOT_IP, ROBOT_PORT) except Exception as e: print("Error connecting to Proxies: {}".format(e)) sys.exit(1) - # Setup ZMQ context = zmq.Context() socket = context.socket(zmq.PUB) socket.bind("tcp://*:{}".format(ZMQ_PORT)) - # Subscribe to Camera - # Resolution: 2 = VGA (640x480), 1 = QVGA (320x240) - # ColorSpace: 11 = RGB, 13 = BGR - # Resolution: 2 = VGA (640x480), 1 = QVGA (320x240) - # ColorSpace: 11 = RGB, 0 = Yuv + # Resolution: 2 = VGA (640x480), 1 = QVGA (320x240). ColorSpace: 11 = RGB, 13 = BGR, 0 = Yuv. resolution = vision_definitions.kQVGA - # OPTIMIZATION: Use YUV Color Space to extract Y (Greyscale) channel - # This reduces bandwidth by 2/3rds (from 3 bytes/pixel to 1 byte/pixel) + # YUV so only the Y (greyscale) channel travels: 1 byte/pixel instead of 3. color_space = vision_definitions.kYuvColorSpace client_name = "zmq_streamer_{}".format(int(time.time())) @@ -69,12 +60,10 @@ def main(): while True: start_time = time.time() - # Get Image # [width, height, layers, colorSpace, timeStamp, binaryImage, cameraID, leftAngle, topAngle, rightAngle, bottomAngle] nao_image = video_proxy.getImageRemote(video_client) - # FPS Calculation (Debug only) - # Source is limited to ~4-5 FPS by hardware + # Debug FPS counter; the source is hardware-limited to ~4-5 FPS. frame_count += 1 if frame_count % 30 == 0: now = time.time() @@ -84,33 +73,11 @@ def main(): last_report_time = now if nao_image: - # NaoQi returns YUV422 when kYuvColorSpace is requested. - # The data is interleaved. However, for bandwidth saving we ONLY want the Y (Luminance) channel. - # In YUV422, Y is every byte?, wait. - # Actually NaoQi returns the raw buffer. - # Checking docs: kYuvColorSpace = 0. - # Image is [Y1, U, Y2, V]. - # Wait, simpler approach: - # If we just send the whole buffer, it's 2 bytes/pixel (YUV422). - # To get 1 byte/pixel we need to extract Y. - # Let's verify buffer size first. + # kYuvColorSpace (0) yields interleaved YUV422 at 2 bytes/pixel. img_data = nao_image[6] - # OPTIMIZATION: Extract Y channel only if YUV - # But actually, NaoQi's ALVideoDevice kYuvColorSpace returns data in YUV422 format. - # Length = Width * Height * 2. - # We want to send Width * Height * 1. - # We can skip every other byte? No, it's YUYV usually. - # Let's stick to sending the RAW YUV422 first (33% saving) to be safe, - # OR if we are confident, strip it. - # Let's send the raw buffer for now, receiving end will handle it. - # Actually, standard is RGB=3 bytes, YUV422=2 bytes. That is a 33% saving immediately. - - # Send via ZMQ - # Topic: "video", Data: raw bytes - # OPTIMIZATION: Extract Y channel only (Greyscale) - # Check for YUYV (153600) vs Already Greyscale (76800) + # Extract Y only: 153600 = QVGA YUYV, 76800 = already greyscale. if len(img_data) == 153600: # YUYV (2 bytes/px) -> Extract Y (Index 0, 2...) y_channel = img_data[0::2] @@ -127,15 +94,9 @@ def main(): # Fallback y_channel = img_data - # Send via ZMQ - # Topic: "video", Data: Timestamp (d) + Image Bytes - # NaoQi timestamp is [seconds, microseconds] - # ts_sec = nao_image[4] - # ts_micro = nao_image[5] - # timestamp = float(ts_sec) + float(ts_micro) / 1000000.0 + # Wire format: topic "video", header (d), then image bytes. - # CRITICAL FIX: NaoQi returns Uptime, Proprioception uses Epoch. - # We overwrite with system time to ensure sync. + # NaoQi stamps uptime (nao_image[4:6]); system time replaces it to match proprioception. timestamp = time.time() # Header: timestamp (double) @@ -143,12 +104,10 @@ def main(): socket.send_multipart(["video", header, y_channel]) - # Sleep to maintain FPS elapsed = time.time() - start_time if elapsed < 1.0 / FPS: time.sleep((1.0 / FPS) - elapsed) - # Log periodic stats if frame_count % 30 == 0: print("Streamer FPS: {:.1f}".format(30.0 / (time.time() - last_report_time))) last_report_time = time.time() diff --git a/tests/unit/test_post_tasks.py b/tests/unit/test_post_tasks.py index d7cc36f..80f1a6e 100644 --- a/tests/unit/test_post_tasks.py +++ b/tests/unit/test_post_tasks.py @@ -1,5 +1,4 @@ import threading -import pytest from post_tasks import TaskRegistry diff --git a/tests/unit/test_shim_server.py b/tests/unit/test_shim_server.py index 56ab80d..ca9b626 100644 --- a/tests/unit/test_shim_server.py +++ b/tests/unit/test_shim_server.py @@ -1,5 +1,4 @@ from unittest.mock import MagicMock -import pytest from shim_server import create_app from post_tasks import TaskRegistry from adapters import build_module_adapters @@ -109,8 +108,7 @@ def test_post_dispatch_returns_task_id(): def test_call_with_omitted_kwargs_uses_empty_dict_default(): - """Verifies the route's data.get('kwargs', {}) default. Callers that - don't send a kwargs key (e.g. curl users) still work.""" + """The route's data.get('kwargs', {}) default lets callers omit the kwargs key.""" client, pepper = _client() resp = client.post("/api/call", json={ "module": "ALMotion", "method": "move", From 4d92f988885de0819a68b2c22d9a962e67ac846b Mon Sep 17 00:00:00 2001 From: jwgcurrie Date: Sun, 26 Jul 2026 17:56:17 +0200 Subject: [PATCH 7/7] de-verbosify comments and docstrings --- py3-naoqi-bridge/audio_publisher.py | 34 ++++------------------- py3-naoqi-bridge/clients/state_client.py | 15 +++------- py3-naoqi-bridge/clients/vision_client.py | 7 ++--- py3-naoqi-bridge/naoqi_proxy.py | 7 ++--- py3-naoqi-bridge/state_service.py | 29 ++++++------------- py3-naoqi-bridge/tests/test_motion.py | 18 ++++-------- setup.sh | 5 +--- src/adapters/laser.py | 3 +- src/adapters/memory.py | 9 ++---- src/adapters/posture.py | 4 +-- src/adapters/tts.py | 6 ++-- src/dispatcher.py | 7 +---- src/driver.py | 5 +--- src/post_tasks.py | 10 ++----- tests/conftest.py | 3 +- tests/run-in-docker.sh | 4 +-- tests/unit/test_dispatcher.py | 2 +- 17 files changed, 43 insertions(+), 125 deletions(-) diff --git a/py3-naoqi-bridge/audio_publisher.py b/py3-naoqi-bridge/audio_publisher.py index 63a0be7..49aa89b 100644 --- a/py3-naoqi-bridge/audio_publisher.py +++ b/py3-naoqi-bridge/audio_publisher.py @@ -1,12 +1,7 @@ # py3-naoqi-bridge/audio_publisher.py (Python 2) """NAOqi ALAudioDevice -> ZMQ PUB :5563. -Subclass of ALModule; processRemote callback runs on a NAOqi dispatcher thread -and enqueues audio buffers. A dedicated publisher thread owns the ZMQ socket -and drains the queue -- ZMQ sockets are not thread-safe, so the NAOqi thread -must never touch the socket directly. - -Mirrors ros-naoqi/naoqi_bridge/naoqi_sensors_py/.../naoqi_microphone.py. +processRemote only enqueues because ZMQ sockets are not thread-safe and the publisher thread owns the socket; mirrors ros-naoqi naoqi_sensors_py/naoqi_microphone.py. """ from __future__ import print_function import os @@ -24,13 +19,7 @@ def _resolve_bind_ip(target_ip, target_port): - """Local IP the kernel would use as source when sending to target. - - Why: ALBroker bound to 0.0.0.0 advertises every local interface to the - parent broker, which then picks one (often a Docker bridge) that is not - routable from the robot. Binding to the specific outbound source IP forces - the robot to dial back on an address it can reach. - """ + """Local source IP for target; ALBroker on 0.0.0.0 advertises every interface and the parent broker may pick an unroutable Docker bridge.""" s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) try: s.connect((target_ip, target_port)) @@ -82,10 +71,7 @@ def run(self): _NAOQI_AVAILABLE = False -# Module-level strong reference. NaoQiModule stores instances as weakrefs; if the -# only reference is a local variable that goes out of scope, the object is GC'd -# and processRemote callbacks silently become no-ops. The variable name MUST -# match the ALModule name string passed to the constructor -- Aldebaran convention. +# Strong module-level reference: NaoQiModule keeps weakrefs, so a local-only one is GC'd and processRemote silently no-ops; the name MUST match the ALModule name string. PepperAudioPub = None @@ -104,9 +90,8 @@ def __init__(self, module_name, publisher, ip, port, bind_ip="0.0.0.0"): self._audio_proxy.subscribe(self.getName()) def processRemote(self, nbOfChannels, nbOfSamplesByChannel, timeStamp, inputBuffer): - """NAOqi audio callback. Docstring is load-bearing: ALModule.autoBind only - registers methods that have a non-empty __doc__, otherwise the robot cannot - invoke this callback and no audio ever arrives.""" + """NAOqi audio callback; ALModule.autoBind registers only methods with a + non-empty __doc__, so emptying this docstring stops all audio delivery.""" self._first_callback_checked = True expected = nbOfChannels * nbOfSamplesByChannel * 2 if len(inputBuffer) != expected: @@ -127,14 +112,7 @@ def shutdown(self): def _probe_reachable(ip, port, timeout_s=3.0): - """Raw TCP probe: returns True if ip:port accepts a connection. - - Used as a fast-fail check before constructing the ALBroker. We avoid - creating an ALProxy here because that would establish a default broker - session; subsequent ALProxy() calls inside the module would bind to it - instead of the broker we explicitly construct, which breaks the callback - routing on real hardware. - """ + """Raw TCP fast-fail probe before the ALBroker is built; an ALProxy here would open a default broker session that later ALProxy calls bind to, breaking callback routing on hardware.""" s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) s.settimeout(timeout_s) try: diff --git a/py3-naoqi-bridge/clients/state_client.py b/py3-naoqi-bridge/clients/state_client.py index 6af95ee..518adbd 100644 --- a/py3-naoqi-bridge/clients/state_client.py +++ b/py3-naoqi-bridge/clients/state_client.py @@ -33,8 +33,7 @@ def run(self): try: if socket.poll(100): topic, data = socket.recv_multipart() - # Unpack: timestamp (double), yaw (float), pitch (float) - # format 'dff' is 16 bytes + # Unpack 'dff' (16 bytes): timestamp double, yaw float, pitch float. if len(data) == 16: ts, yaw, pitch = struct.unpack('dff', data) with self.lock: @@ -47,10 +46,7 @@ def run(self): context.term() def get_state_at(self, query_time): - """ - Returns (yaw, pitch) interpolated at query_time. - Returns None if query_time is too old or too new (out of buffer range). - """ + """Returns (yaw, pitch) interpolated at query_time, or None when query_time predates the buffer.""" with self.lock: if not self.buffer: return None @@ -60,13 +56,11 @@ def get_state_at(self, query_time): # Check bounds (allowing 50ms slack) if query_time < timestamps[0] - 0.05: - # Too old return None if query_time > timestamps[-1] + 0.05: - # Too new (future?) - return self.buffer[-1][1:] # Return latest + # Too new: clamp to the latest sample. + return self.buffer[-1][1:] - # Find insertion point idx = bisect.bisect_right(timestamps, query_time) if idx == 0: @@ -74,7 +68,6 @@ def get_state_at(self, query_time): if idx == len(timestamps): return self.buffer[-1][1:] - # Interpolate t0, y0, p0 = self.buffer[idx-1] t1, y1, p1 = self.buffer[idx] diff --git a/py3-naoqi-bridge/clients/vision_client.py b/py3-naoqi-bridge/clients/vision_client.py index 51de6e0..c732036 100644 --- a/py3-naoqi-bridge/clients/vision_client.py +++ b/py3-naoqi-bridge/clients/vision_client.py @@ -12,7 +12,7 @@ def __init__(self, streamer_uri="tcp://localhost:5559"): self.running = False self.lock = threading.Lock() self.callback = None - self.daemon = True # Daemonize thread to kill it with main process + self.daemon = True def start_receiving(self, callback): """Register a callback(timestamp, img_bgr) to be called on new frames.""" @@ -41,7 +41,7 @@ def run(self): while self.running: try: - # DRAIN QUEUE: Read all available frames, keep only the last one + # Drain the queue, keeping only the newest frame. last_msg = None while socket.poll(0): last_msg = socket.recv_multipart() @@ -66,8 +66,7 @@ def run(self): # Invalid msg continue - # Decode - # Assume QVGA (320x240) YUV422 or Grey + # Decode assumes QVGA (320x240) YUV422 or Grey. w, h = 320, 240 img_bgr = None diff --git a/py3-naoqi-bridge/naoqi_proxy.py b/py3-naoqi-bridge/naoqi_proxy.py index 24638a4..69b2417 100644 --- a/py3-naoqi-bridge/naoqi_proxy.py +++ b/py3-naoqi-bridge/naoqi_proxy.py @@ -10,10 +10,7 @@ class NaoqiProxyError(Exception): class SimStubWarning(UserWarning): - """Triggered when the sim shim returns a stub (no-op) response and the - client has opted into stub-aware warnings via `warn_on_stubs=True` or - the `NAOQI_SIM_WARN_STUBS=1` env var.""" - + """Triggered when the sim shim returns a stub (no-op) response and the client opted in via `warn_on_stubs=True` or the `NAOQI_SIM_WARN_STUBS=1` env var.""" class NaoqiModule: def __init__(self, client, module_name): @@ -81,7 +78,7 @@ def _call_shim(self, module, method, args, kwargs): except json.JSONDecodeError: return False, None, "Failed to decode JSON response from shim server." except SimStubWarning: - # Let SimStubWarning propagate even if it's been turned into an errorbby the warnings filter. + # Propagate SimStubWarning even when the warnings filter raises it as an error. raise except Exception as e: return False, None, f"An unexpected error occurred: {e}" diff --git a/py3-naoqi-bridge/state_service.py b/py3-naoqi-bridge/state_service.py index c0605de..c35c1ed 100644 --- a/py3-naoqi-bridge/state_service.py +++ b/py3-naoqi-bridge/state_service.py @@ -8,7 +8,6 @@ import math from naoqi import ALProxy -# --- Configuration Loader --- import argparse RATE_JOINTS = 50.0 # 50Hz for control loop @@ -28,8 +27,7 @@ def get_config(): return args def get_proximity_keys(): - # --- 1. Sonar --- - # Try Modern 2.5 keys first, then Platform fallback + # Sonar: NAOqi 2.5 keys first, Platform keys as fallback. sonar_keys = [ "Device/SubDeviceList/US/FrontLeft/Sensor/Value", "Device/SubDeviceList/US/FrontRight/Sensor/Value", @@ -41,21 +39,19 @@ def get_proximity_keys(): "Device/SubDeviceList/Platform/Back/Sonar/Sensor/Value" ] - # --- 2. Lasers --- - # Traditional Laser + # Traditional laser: 15 horizontal values per side. laser_keys = [] for loc in ["Front", "Left", "Right"]: for i in range(15): laser_keys.append("Device/SubDeviceList/Laser/{}/Horizontal/Sensor/Value{}".format(loc, i)) - # Platform Segment Laser (Found on some specific firmwares) + # Platform segment laser, present on some firmwares only. laser_platform_keys = [] for loc in ["Front", "Left", "Right"]: for i in range(1, 16): laser_platform_keys.append("Device/SubDeviceList/Platform/LaserSensor/{}/Horizontal/Seg{:02d}/X/Sensor/Value".format(loc, i)) laser_platform_keys.append("Device/SubDeviceList/Platform/LaserSensor/{}/Horizontal/Seg{:02d}/Y/Sensor/Value".format(loc, i)) - # --- 3. Bumpers --- bumper_keys = [ "Device/SubDeviceList/Platform/FrontLeft/Bumper/Sensor/Value", "Device/SubDeviceList/Platform/FrontRight/Bumper/Sensor/Value", @@ -67,7 +63,6 @@ def get_proximity_keys(): def main(): args = get_config() - # Connect to NaoQi try: motion_proxy = ALProxy("ALMotion", args.ip, args.port) memory_proxy = ALProxy("ALMemory", args.ip, args.port) @@ -75,7 +70,6 @@ def main(): print("Error connecting to Naoqi: {}".format(e)) sys.exit(1) - # Setup ZMQ context = zmq.Context() socket = context.socket(zmq.PUB) socket.bind("tcp://*:{}".format(args.zmq_port)) @@ -90,7 +84,7 @@ def main(): start_loop = time.time() loop_count += 1 - # --- 1. JOINT DATA (50Hz) --- + # Joint data on topic "joints" at 50Hz. try: names = ["HeadYaw", "HeadPitch"] angles = motion_proxy.getAngles(names, True) @@ -101,11 +95,10 @@ def main(): except Exception as e: print("Joints Error: {}".format(e)) - # --- 2. PROXIMITY DATA (~15Hz) --- + # Proximity data on topic "proximity" at ~15Hz. if loop_count % PROXIMITY_DIVIDER == 0: payload = {"timestamp": start_loop, "sonar": {}, "lasers": {}, "bumpers": {}} - # A. SONAR try: s_vals = memory_proxy.getListData(prox_sonar_keys) if s_vals and s_vals[0] is not None: @@ -114,10 +107,9 @@ def main(): "back_left": s_vals[2], "back_right": s_vals[3] } else: - # Try fallback s_vals = memory_proxy.getListData(prox_sonar_fallback) if s_vals and s_vals[0] is not None: - # Map 2 sonars to 4 fields (approx) + # Map the 2 fallback sonars onto 4 fields (approx). payload["sonar"] = { "front_left": s_vals[0], "front_right": s_vals[0], "back_left": s_vals[1], "back_right": s_vals[1] @@ -125,7 +117,6 @@ def main(): except Exception: pass - # B. LASERS try: l_vals = memory_proxy.getListData(prox_laser_keys) if l_vals and l_vals[0] is not None: @@ -133,11 +124,9 @@ def main(): "front": l_vals[0:15], "left": l_vals[15:30], "right": l_vals[30:45] } else: - # Try Platform/Segment laser l_vals = memory_proxy.getListData(prox_laser_platform) if l_vals and l_vals[0] is not None: - # We have X,Y pairs for each segment. Convert to distance. - # l_vals format: [F1X, F1Y, F2X, F2Y, ..., L1X, L1Y, ..., R1X, R1Y, ...] + # X,Y pairs per segment converted to distance; l_vals is [F1X, F1Y, F2X, F2Y, ..., L1X, L1Y, ..., R1X, R1Y, ...]. def compute_dists(start_idx): dists = [] for i in range(15): @@ -156,7 +145,6 @@ def compute_dists(start_idx): sys.stderr.write("Laser Error: {}\n".format(e)) sys.stderr.flush() - # C. BUMPERS try: b_vals = memory_proxy.getListData(prox_bumper_keys) if b_vals: @@ -167,10 +155,9 @@ def compute_dists(start_idx): except Exception: pass - # Publish what we have socket.send_multipart(["proximity", json.dumps(payload)]) - # --- Sleep to maintain rate --- + # Sleep to hold the 50Hz joint rate. elapsed = time.time() - start_loop if elapsed < dt_joints: time.sleep(dt_joints - elapsed) diff --git a/py3-naoqi-bridge/tests/test_motion.py b/py3-naoqi-bridge/tests/test_motion.py index 4060bf2..58685f5 100644 --- a/py3-naoqi-bridge/tests/test_motion.py +++ b/py3-naoqi-bridge/tests/test_motion.py @@ -1,23 +1,17 @@ -# test_motion.py import os import sys -# Add the parent directory to the path to find the 'naoqi_proxy' module +# Parent directory on the path so 'naoqi_proxy' imports. sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))) from naoqi_proxy import NaoqiClient, NaoqiProxyError -# --- Connection Settings --- -# The shim server runs on localhost inside the container. -# The Robot IP is configured in the shim_server's environment. +# Shim runs on localhost inside the container; robot IP comes from its environment. NAOQI_SHIM_IP = "127.0.0.1" NAOQI_SHIM_PORT = 5000 def run_test(): - """ - Connects to the Naoqi proxy and attempts to call methods - that previously failed with nested arguments. - """ + """Calls Naoqi proxy methods that take nested arguments.""" print("--- Starting Naoqi proxy test ---") print("Connecting to Shim Server: {}:{}".format(NAOQI_SHIM_IP, NAOQI_SHIM_PORT)) print("Target Robot IP is configured in the shim server's environment (NAOQI_IP).") @@ -29,8 +23,7 @@ def run_test(): client = NaoqiClient(host=NAOQI_SHIM_IP, port=NAOQI_SHIM_PORT) print("Successfully connected to proxy.") - # --- Test 1: ALMotion.setBreathConfig --- - # This call uses a list of lists, which previously failed. + # setBreathConfig takes a list of lists. breath_config = [ ['Bpm', 15.0], ['Amplitude', 0.9] @@ -39,8 +32,7 @@ def run_test(): client.ALMotion.setBreathConfig(breath_config) print("ALMotion.setBreathConfig call succeeded.") - # --- Test 2: ALMotion.setAngles --- - # This call uses a list of joint names (strings). + # setAngles takes a list of joint-name strings. joint_names = ["RShoulderPitch", "RShoulderRoll"] angles = [0.5, 0.5] speed = 0.1 diff --git a/setup.sh b/setup.sh index 7673237..904215e 100755 --- a/setup.sh +++ b/setup.sh @@ -1,8 +1,5 @@ #!/usr/bin/env bash -# Prepare PepperBox for physical-robot use by ensuring the SoftBank Robotics -# pynaoqi SDK is available locally. The SDK is proprietary and never built into -# the image; it lives under $HOME/.pepperbox on the user's machine and is -# bind-mounted into the container at runtime. +# Prepare PepperBox for physical-robot use: the proprietary pynaoqi SDK is never in the image, it lives under $HOME/.pepperbox and is bind-mounted at runtime. # # Sim-only users do not need this script; qibullet is already in the image. # diff --git a/src/adapters/laser.py b/src/adapters/laser.py index 236fc9e..fb00ee9 100644 --- a/src/adapters/laser.py +++ b/src/adapters/laser.py @@ -1,5 +1,4 @@ -"""ALLaser adapter. PepperBox-custom NAOqi module name; maps to qibullet's -laser debug-line visualisation.""" +"""ALLaser adapter: a PepperBox-invented NAOqi module name mapped to qibullet's laser debug lines.""" from .base import GenericAdapter diff --git a/src/adapters/memory.py b/src/adapters/memory.py index 8aa970f..6c19238 100644 --- a/src/adapters/memory.py +++ b/src/adapters/memory.py @@ -1,6 +1,4 @@ -"""ALMemory adapter. Key-routed getData translates NAOqi's stringly-typed -sensor lookups to qibullet's laser API. Most ALMemory operations have no -qibullet equivalent and stub to None or the closest sensible value.""" +"""ALMemory adapter: getData routes NAOqi's stringly-typed sensor keys to qibullet's laser API, and anything unmodelled stubs to None.""" from .base import GenericAdapter, stub @@ -8,14 +6,13 @@ _SONAR_BACK = "Device/SubDeviceList/Platform/Back/Sonar/Sensor/Value" _LASER_FRONT = "Device/SubDeviceList/Platform/Laser/Front/Sensor/Value" _LANDMARK = "LandmarkDetected" -_MAX_SONAR_RANGE = 3.0 # meters +_MAX_SONAR_RANGE = 3.0 # meters class ALMemoryAdapter(GenericAdapter): def getData(self, key, *_rest): - # NAOqi's ALMemory.getData accepts an optional 2nd arg (timeout/format). - # Capture it as *_rest and ignore. + # NAOqi's getData takes an optional 2nd arg (timeout/format); ignored here. if key == _SONAR_FRONT: return self._min_laser(self._pepper.getFrontLaserValue()) if key == _SONAR_BACK: diff --git a/src/adapters/posture.py b/src/adapters/posture.py index bd04417..93cca00 100644 --- a/src/adapters/posture.py +++ b/src/adapters/posture.py @@ -1,6 +1,4 @@ -"""ALRobotPosture adapter. Direct passthrough to qibullet's goToPosture, -which returns False for postures qibullet doesn't model (Sit, SitRelax, -LyingBelly, LyingBack).""" +"""ALRobotPosture adapter: passthrough to qibullet's goToPosture, which returns False for Sit, SitRelax, LyingBelly and LyingBack.""" from .base import GenericAdapter diff --git a/src/adapters/tts.py b/src/adapters/tts.py index ccdd75a..3c6fa12 100644 --- a/src/adapters/tts.py +++ b/src/adapters/tts.py @@ -1,6 +1,4 @@ -"""ALTextToSpeech adapter. qibullet has no audio backend. Calls to .say -log the requested phrase to stderr so a developer watching the shim's -docker logs sees the intended utterance.""" +"""ALTextToSpeech adapter: qibullet has no audio backend, so `say` logs the phrase to stderr.""" import sys @@ -14,6 +12,6 @@ def say(self, text): sys.stderr.write("[SIM-TTS] {}\n".format(text)) return None - @stub(default=["English"]) + @stub() def getAvailableLanguages(self): return ["English"] diff --git a/src/dispatcher.py b/src/dispatcher.py index 486278d..f1112cb 100644 --- a/src/dispatcher.py +++ b/src/dispatcher.py @@ -1,11 +1,6 @@ """Dispatcher: looks up the per-module adapter and forwards calls. -Returns (result, is_stub) so the Flask layer can set the X-Sim-Stub header -without inspecting adapter internals. - -Raises AttributeError for unknown modules, unknown methods on known -modules, blocked names, and private names. The shim's route handler turns -AttributeError into HTTP 500 with the exception's message as the error body. +Returns (result, is_stub) for the X-Sim-Stub header, and raises AttributeError for unknown, blocked or private names, which the route turns into HTTP 500 carrying the message. """ diff --git a/src/driver.py b/src/driver.py index 589859c..f075a33 100644 --- a/src/driver.py +++ b/src/driver.py @@ -1,7 +1,4 @@ -"""This is the qibullet sim lifecycle owner. - -Holds the simulation and the Pepper instance the adapters dispatch against. -Adapters call into `self.pepper` directly.""" +"""qibullet sim lifecycle owner: holds the simulation and the `pepper` instance adapters dispatch against.""" from qibullet import SimulationManager diff --git a/src/post_tasks.py b/src/post_tasks.py index e2e393b..f0cca7b 100644 --- a/src/post_tasks.py +++ b/src/post_tasks.py @@ -1,12 +1,6 @@ """Task registry for the sim shim's `post` dispatch. -NAOqi's `post` returns a task ID that callers later query via `wait` or -`getCurrentPosition`. qibullet has no async dispatch, so we run targets -synchronously and treat the resulting state as "already done". - -Monotonic int IDs (never reused) so a slow caller's stale task_id never -aliases to a new task. LRU-bounded storage (default 100k) caps memory in -long-running sim sessions. +qibullet has no async dispatch, so `submit_sync` runs targets synchronously and reports "already done" to `wait`, with monotonic never-reused int IDs in LRU-bounded storage (default 100k). """ import threading @@ -15,7 +9,7 @@ class TaskRegistry: MAX_ENTRIES = 100_000 - # LRU-bounded storage is hardcoded and not tunable from in config. Not a priority until this is blocking. + # Hardcoded; making it config-tunable is deferred until it blocks something. def __init__(self): self._lock = threading.Lock() diff --git a/tests/conftest.py b/tests/conftest.py index b735a36..4cefc08 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -1,8 +1,7 @@ import os import sys -# Add `src/` to sys.path so imports work in test files. Matches the -# runtime invocation pattern: `python3 src/shim_server.py`. +# Add `src/` to sys.path, matching the runtime invocation `python3 src/shim_server.py`. _SRC = os.path.abspath( os.path.join(os.path.dirname(__file__), os.pardir, "src") ) diff --git a/tests/run-in-docker.sh b/tests/run-in-docker.sh index 17b8a32..7cfab6a 100755 --- a/tests/run-in-docker.sh +++ b/tests/run-in-docker.sh @@ -1,9 +1,7 @@ #!/bin/bash # Run pytest inside the pepper-box container. # -# Mounts the working tree (src/, tests/, py3-naoqi-bridge/) over the image's -# copies so that we test the current code. Installs pytest at startup -# because the runtime image keeps test deps out. +# Mounts the working tree (src/, tests/, py3-naoqi-bridge/) over the image's copies and installs pytest, which the runtime image omits. # # Usage: ./tests/run-in-docker.sh [pytest-args...] # ./tests/run-in-docker.sh tests/unit/ -v diff --git a/tests/unit/test_dispatcher.py b/tests/unit/test_dispatcher.py index 0d546b1..4ae5649 100644 --- a/tests/unit/test_dispatcher.py +++ b/tests/unit/test_dispatcher.py @@ -6,7 +6,7 @@ def _dispatcher(): - # Create a mock with spec of known methods so hasattr works correctly. + # spec lists the known pepper methods so hasattr answers correctly. pepper = MagicMock(spec=[ 'move', 'moveToward', 'moveTo', 'stopMove', 'getPosition', 'getAnglesPosition', 'setAngles', 'getRobotPosition',