Johnliu/optitrack emulation

.agents/skills/add-unit-tests/SKILL.md

@@ -35,11 +35,12 @@ robot/ros_ws/src/<layer>/<package>/
 └── CMakeLists.txt              # wires ament_add_gtest under BUILD_TESTING
 
 tests/robot/<layer>/<package>/
-└── test_<name>.py              # ← thin PROXY (re-exports tests from above)
+└── test_<name>.py              # ← thin PROXY (registers tests from above)
 ```
 
-The **proxy** is a one-file shim that loads the real test module with `importlib`
-and re-exports every `test_*` function. This means:
+The **proxy** is a one-file shim that calls ``register_unit_tests()`` to load the
+real test module with ``importlib`` and expose every ``test_*`` function to pytest.
+This means:
 
 | Invocation | What runs |
 |---|---|
@@ -48,6 +49,39 @@ and re-exports every `test_*` function. This means:
 | CI `system-tests.yml` (PR open / approved) | Same path via `pytest tests/` |
 | `colcon test --packages-select <pkg>` | Real test in `package/test/` directly |
 
+### `register_unit_tests` (in `tests/conftest.py`)
+
+Proxies call this helper; they do not duplicate test logic. Signature:
+
+```python
+register_unit_tests(target_globals, test_dir, *module_files)
+```
+
+| Argument | Meaning |
+|---|---|
+| `target_globals` | Pass ``globals()`` from the proxy module — pytest collects ``test_*`` names injected here |
+| `test_dir` | Co-located test directory, usually ``repo_path("robot/ros_ws/src/<layer>/<pkg>/test")`` |
+| `*module_files` | One or more filenames under ``test_dir`` (e.g. ``"test_foo.py"``); fold several into one proxy |
+
+**What it does (in order):**
+
+1. Prepends ``test_dir`` and ``test_dir.parent`` (package or extension root) to
+   ``sys.path`` so loaded modules can import production code and sibling helpers.
+2. Loads each file via ``importlib.util.spec_from_file_location`` under a
+   synthetic name (``_unit_<parent>_<stem>``) so proxy and source can share the
+   same basename without circular imports.
+3. Copies every ``test_*`` callable from the loaded module into ``target_globals``.
+4. Wraps each with ``pytest.mark.unit`` so ``pytest tests/ -m unit`` selects them
+   even if the source omitted ``pytestmark``.
+
+**What it does not do:** run tests, install packages, or replace ``colcon test``.
+For local iteration against source only, run ``pytest <package>/test/`` (add a tiny
+``conftest.py`` in that dir for ``sys.path`` — see the emulator example below).
+
+Pair with ``repo_path()`` from the same conftest — paths are anchored on
+``AIRSTACK_ROOT`` (CI export, repo root locally). **Never** use
+``Path(__file__).parents[N]`` in a proxy.
+
 ## Step-by-Step: Adding a Python Unit Test
 
 ### 1. Identify pure-Python logic to test
@@ -114,50 +148,34 @@ For `rclpy.node.Node` subclasses use a real dummy base class instead of a
 
 ### 3. Write the thin proxy in tests/robot/
 
-Create `tests/robot/<layer>/<package>/test_<name>.py`:
+Create `tests/robot/<layer>/<package>/test_<name>.py`. Use
+``register_unit_tests`` + ``repo_path`` from ``tests/conftest.py`` so the proxy
+stays a two-call shim:
 
 ```python
 # Copyright (c) 2024 Carnegie Mellon University
 # MIT License - see LICENSE in the repository root for full text.
-"""Proxy: re-exposes <package> unit tests from the package source tree.
+"""Proxy: registers <package> unit tests from the package source tree.
 
-Unit test logic lives co-located with the package source (ROS 2 / colcon convention):
+Unit test logic lives co-located with the package (ROS 2 / colcon convention):
   robot/ros_ws/src/<layer>/<package>/test/test_<name>.py
 
-This file makes those tests discoverable by ``pytest tests/`` (CI) and
-``airstack test -m unit`` without any changes to the CI workflow.
+Discoverable by ``pytest tests/`` (CI) and ``airstack test -m unit``.
 """
-import importlib.util
-import sys
-from pathlib import Path
-
-_repo_root = Path(__file__).resolve().parents[N]   # adjust N so this resolves to repo root
-_pkg_test = _repo_root / "robot/ros_ws/src/<layer>/<package>/test"
-_real_file = _pkg_test / "test_<name>.py"
+from conftest import register_unit_tests, repo_path
 
-# If the test imports from a package module, ensure the package root is on sys.path.
-# Example: _pkg_root = _pkg_test.parent; sys.path.insert(0, str(_pkg_root))
-
-# Load the real module under a unique name to avoid the circular import that
-# would occur if we used `from test_<name> import *` (this file has the same
-# name, and pytest adds its directory to sys.path at collection time).
-_spec = importlib.util.spec_from_file_location("_<package>_unit_tests", _real_file)
-_real = importlib.util.module_from_spec(_spec)
-_spec.loader.exec_module(_real)
-
-# Re-export every test_* symbol so pytest collects them from this proxy.
-for _name in dir(_real):
-    if _name.startswith("test_"):
-        globals()[_name] = getattr(_real, _name)
+register_unit_tests(
+    globals(),
+    repo_path("robot/ros_ws/src/<layer>/<package>/test"),
+    "test_<name>.py",   # pass several filenames to fold multiple modules into one proxy
+)
 ```
 
-**Counting `parents[N]` to reach the repo root:**
-
-| Proxy location | `parents[N]` for repo root |
-|---|---|
-| `tests/robot/<layer>/<package>/` | `parents[4]` |
-| `tests/sim/<tool>/` | `parents[3]` |
-| `tests/gcs/<package>/` | `parents[3]` |
+For a **direct** `pytest <package>/test/` (or `colcon test`) run — which bypasses
+the proxies — add a tiny `conftest.py` in the package `test/` dir that puts the
+package/extension root on `sys.path` (see
+`simulation/.../optitrack.natnet.emulator/test/conftest.py`). The test source
+then stays free of `sys.path` boilerplate.
 
 ### 4. Ensure the tests/ directory structure exists
 
@@ -257,16 +275,16 @@ there are listed in [`tests/colcon_unit_test_packages.yaml`](../../../tests/colc
 
 The same proxy pattern applies verbatim:
 
-**Sim-side Python** (e.g. motive emulator protocol logic):
+**Sim-side Python** (e.g. OptiTrack NatNet emulator):
 ```
 simulation/.../<tool>/test/test_<name>.py   ← source
-tests/sim/<tool>/test_<name>.py             ← proxy (parents[3] = repo root)
+tests/sim/<tool>/test_<name>.py             ← proxy (register_unit_tests + repo_path)
 ```
 
 **GCS modules**:
 ```
 gcs/.../<pkg>/test/test_<name>.py           ← source
-tests/gcs/<pkg>/test_<name>.py             ← proxy (parents[3] = repo root)
+tests/gcs/<pkg>/test_<name>.py             ← proxy (register_unit_tests + repo_path)
 ```
 
 `pytest tests/ -m unit` discovers them through the proxy without any
@@ -280,7 +298,8 @@ pytest.ini or CI changes needed.
 |---|---|
 | Where does test source live? | `<component>/…/<package>/test/` (co-located with the package) |
 | Where does pytest discover tests? | `tests/robot/` (or `tests/sim/`, `tests/gcs/`) via thin proxy |
-| How does the proxy avoid circular import? | `importlib.util.spec_from_file_location` with a unique module name |
+| How does the proxy register tests? | ``register_unit_tests(globals(), repo_path(...), "test_*.py")`` in `tests/conftest.py` |
+| How does the proxy avoid circular import? | `importlib.util.spec_from_file_location` with a unique synthetic module name |
 | What mark do all unit tests use? | `@pytest.mark.unit` |
 | What CI workflow runs them? | `system-tests.yml` — runs `pytest tests/` which includes unit tests |
 | When does that workflow trigger? | PR opened, `/pytest` comment, `workflow_dispatch` |
@@ -302,8 +321,9 @@ Corresponding proxies: `tests/robot/perception/natnet_ros2/test_natnet_ros2.py`,
 ## Files to Know
 
 - `.github/workflows/system-tests.yml` — CI workflow (runs `pytest tests/` including unit tests)
+- `tests/conftest.py` — `register_unit_tests`, `repo_path`, shared fixtures
 - `tests/pytest.ini` — mark registration (`unit`, `build_docker`, etc.)
 - `tests/robot/` — proxy layer mirroring `robot/ros_ws/src/`
-- `tests/sim/` — proxy layer for sim-side code (future)
+- `tests/sim/` — proxy layer for sim-side extensions and tools
 - `tests/gcs/` — proxy layer for GCS code (future)
 - `tests/README.md` — full test harness reference

.agents/skills/docker-build-profiles/SKILL.md

@@ -7,12 +7,14 @@ Summary
 When to use
 - When adding or updating a `docker-compose` profile that passes `PYTHON_VERSION`, `ROS_DISTRO`, or other numeric-like build args.
 - When an automated agent needs to verify a new profile will produce a correct `PYTHONPATH` and avoid YAML float-parsing bugs.
+- When adding or reviewing platform-specific robot builds that need a non-default ROS library architecture suffix in `LD_LIBRARY_PATH`.
 
 Actions the agent can perform
 1. Validate `docker-compose.yaml` args are quoted when numeric-like (e.g. `PYTHON_VERSION: "3.10"`).
 2. Insert a build-time validation `RUN` into `robot/docker/Dockerfile.robot` to fail early when the ROS Python path does not exist.
-3. Add or update a short test in documentation showing how to build the `builder` stage and check `ament_package` import.
-4. Suggest `network: host` under `build:` for L4T/Jetson profiles only when necessary (kernel iptables workarounds).
+3. Ensure `TARGET_ARCH` is passed as a `build.args` value, not a runtime `environment` value, when a profile needs a non-default ROS library path (default is `x86_64`; arm64 targets such as VOXL and L4T/Jetson should use `aarch64`).
+4. Add or update a short test in documentation showing how to build the `builder` stage and check `ament_package` import.
+5. Suggest `network: host` under `build:` for L4T/Jetson profiles only when necessary (kernel iptables workarounds).
 
 Snippets (copyable)
 
@@ -43,12 +45,14 @@ docker run --rm -it airstack-builder-test:local bash -c "python3 -c 'import amen
 Guidance for agents when editing the repo
 - Prefer making minimal, reversible changes: add the `RUN test -d ...` check early in the Dockerfile and gate it with informative message text.
 - When updating `docker-compose.yaml`, only quote the numeric-like values; do not change unrelated fields.
+- Put `TARGET_ARCH` under `build.args`; setting it under `environment` is too late because `LD_LIBRARY_PATH` is baked into the image by `Dockerfile.robot`.
 - If creating PRs, include a short note in the PR description instructing maintainers to run the builder-stage sanity build on both an amd64 desktop profile and an arm64 L4T profile.
 
 Troubleshooting notes
 - YAML quirk: unquoted `3.10` may be parsed as float `3.1` — this changes path strings and breaks imports (e.g., `python3.1` instead of `python3.10`).
 - Jetson/L4T builds may require `network: host` during the build to avoid kernel iptables/raw table missing-module errors.
 - Jetson **`robot-l4t`** builds from **`robot-l4t-stack-base`** (`robot/docker/Dockerfile.l4t-stack-base`), not raw dustynv, so **`Dockerfile.robot` stays Ubuntu-shaped.** `airstack image-build --profile l4t robot-l4t` triggers **`robot-l4t-stack-base`** first (`airstack.sh`); bare `compose build robot-l4t` can still parallelize badly, so list stack-base explicitly if not using AirStack CLI.
+- `Dockerfile.robot` defaults `TARGET_ARCH=x86_64` for desktop builds. Arm64 services (for example, VOXL and Jetson/L4T) must pass `TARGET_ARCH: aarch64` in `build.args` so the ROS library path includes `/opt/ros/${ROS_DISTRO}/lib/aarch64-linux-gnu`.
 
 Examples of agent prompts
 - "Check `robot/docker/docker-compose.yaml` for `PYTHON_VERSION` entries and quote any unquoted numeric values; open a PR with the fixes and include a test log from a builder-stage build."
@@ -77,6 +81,7 @@ This section shows the minimal, recommended steps an agent or maintainer should
 
   - Add a service block in `robot/docker/docker-compose.yaml` (or an override file) and set `build.args` for the profile.
   - Always quote `PYTHON_VERSION` values (e.g. `"3.10"`) so YAML does not convert them to floats.
+  - Leave `TARGET_ARCH` unset for x86-64 desktop builds. Set `TARGET_ARCH: aarch64` under `build.args` for arm64 builds so `Dockerfile.robot` composes the correct ROS `LD_LIBRARY_PATH`.
 
   Example snippet to add:
 
@@ -89,6 +94,7 @@ This section shows the minimal, recommended steps an agent or maintainer should
       BASE_IMAGE: nvcr.io/nvidia/l4t-jetpack:r36.4.0
       ROS_DISTRO: humble
       PYTHON_VERSION: "3.10"
+      TARGET_ARCH: aarch64
       REAL_ROBOT: true
       SKIP_MACVO: true
      # for L4T builds only when necessary