Chore/refactor

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.
 

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,20 +33,15 @@ else
         cat >&2 <<EOF
 [Entrypoint] ERROR: pynaoqi SDK not mounted
 
-The physical-robot bridge needs SoftBank Robotics' pynaoqi mounted at
-/opt/pynaoqi-python2.7-2.5.7.1-linux64 inside the container. Default host
-location is \$HOME/.pepperbox/pynaoqi-python2.7-2.5.7.1-linux64, populated by
-running ./setup.sh on the host.
+The physical-robot bridge needs SoftBank Robotics' pynaoqi mounted at /opt/pynaoqi-python2.7-2.5.7.1-linux64 inside the container. The default host location is \$HOME/.pepperbox/pynaoqi-python2.7-2.5.7.1-linux64, populated by running ./setup.sh on the host.
 
 Looked for: $NAOQI_PY
 EOF
         exit 1
     fi
 
     NAOQI_PORT=${NAOQI_PORT:-9559}
-    # 20s tolerance: Pepper's onboard 2.4 GHz WiFi can introduce 5-50ms RTT
-    # variance and intermittent packet loss at any moment. A short timeout
-    # produces spurious pre-flight failures on real-world lab conditions.
+    # 20s tolerance: Pepper's 2.4 GHz WiFi adds 5-50ms RTT variance and packet loss, so a short timeout fails pre-flight spuriously.
     if ! python2 -c "
 import socket, sys
 s = socket.socket()
@@ -61,8 +55,7 @@ except Exception as exc:
         cat >&2 <<EOF
 [Entrypoint] ERROR: cannot reach NAOqi at $NAOQI_IP:$NAOQI_PORT
 
-Check: robot powered on, on the same network, NAOQI_IP / NAOQI_PORT correct
-in robot.env, and no firewall between the host and the robot.
+Check: robot powered on, on the same network, NAOQI_IP / NAOQI_PORT correct in robot.env, and no firewall between the host and the robot.
 EOF
         exit 1
     fi

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.

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:

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,21 +56,18 @@ 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:
                 return self.buffer[0][1:]
             if idx == len(timestamps):
                 return self.buffer[-1][1:]
 
-            # Interpolate
             t0, y0, p0 = self.buffer[idx-1]
             t1, y1, p1 = self.buffer[idx]
 

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
 

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}"