mass code cleaning

Author: JohnYanxinLiu

.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,39 +148,29 @@ 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`. Use the shared
-`reexport_unit_tests` + `repo_path` helpers from `tests/conftest.py` so the proxy
-stays a two-call shim and the cross-tree path is anchored on `AIRSTACK_ROOT`
-(exported by CI, defaults to the repo root locally) — **never** count
-`Path(__file__).parents[N]` or hardcode `sys.path` walks in the proxy:
+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``.
 """
-from conftest import reexport_unit_tests, repo_path
+from conftest import register_unit_tests, repo_path
 
-reexport_unit_tests(
+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
 )
 ```
 
-`reexport_unit_tests` (in `tests/conftest.py`) execs each co-located module with
-`importlib` under a unique name (avoiding the same-filename circular import), puts
-both the test dir and its parent (the package root) on `sys.path` so the source
-can import its package and sibling helpers, and tags every re-exported `test_*`
-with `pytest.mark.unit`. Because the root `conftest` is imported before any proxy
-is collected, `from conftest import ...` resolves in both CI and local runs.
-
 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
@@ -251,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
@@ -274,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` |
@@ -296,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/optitrack-development/SKILL.md

@@ -39,7 +39,7 @@ flowchart LR
 | Robot client | [`robot/ros_ws/src/perception/natnet_ros2/`](../../../robot/ros_ws/src/perception/natnet_ros2/) | ROS 2 node; uses **official NatNet SDK** (`NatNetClient::Connect`) |
 | SDK install | `natnet_ros2/lib/libNatNet.so`, `include/natnet/` | Download via `airstack setup --natnet` (proprietary, not in git) |
 | Emulator (WIP) | [`simulation/isaac-sim/extensions/optitrack.natnet.emulator/`](../../../simulation/isaac-sim/extensions/optitrack.natnet.emulator/) | Python NatNet **server** for sim / integration tests |
-| Integration tests | [`tests/integration/natnet/README.md`](../../../tests/integration/natnet/README.md) | End-to-end UDP tests against real SDK parser (marks: `integration`, `natnet`) |
+| Integration tests | [`tests/integration/natnet/README.md`](../../../tests/integration/natnet/README.md) | End-to-end UDP tests against real SDK parser (mark: `integration`) |
 
 **Enable on robot:** `LAUNCH_NATNET=true` in `.env` → [`perception.launch.xml`](../../../robot/ros_ws/src/perception/perception_bringup/launch/perception.launch.xml) includes `natnet_ros2.launch.py`.
 
@@ -198,7 +198,7 @@ Use the SDK's `NatNetTypes.h` and `PacketClient.cpp` for on-wire layouts — not
 |-------|----------|-----------|
 | Unit (no network) | `test_natnet_logic.cpp`, `FakeNatNetClient` | Negotiation logic, topic names |
 | Protocol capture | Minimal client + UDP stub (see above) | Wire-format `NAT_CONNECT`, client endpoint model |
-| Integration | `tests/integration/natnet/` | Full SDK parser + `natnet_ros2_node` (marks: `integration`, `natnet`) |
+| Integration | `tests/integration/natnet/` | Full SDK parser + `natnet_ros2_node` (mark: `integration`) |
 | System (future) | `airstack test -m sensors` | Topic Hz on `/perception/optitrack/...` |
 
 ```bash

.agents/skills/run-system-tests/SKILL.md

@@ -25,7 +25,7 @@ This skill is about the **test harness itself** — pytest marks, fixtures, the
 The suite lives at `tests/` (repo root) and is fully pytest-based. Configuration is in `tests/pytest.ini` and shared infrastructure in `tests/conftest.py`.
 
 - **`tests/system/`** — Docker stack integration tests. Marks: `build_docker`, `build_packages`, `liveliness`, `sensors`, `takeoff_hover_land`.
-- **`tests/robot/`** and **`tests/sim/`** — Hermetic **unit** tests (`@pytest.mark.unit`). These are **thin proxy files** that re-export tests from each ROS 2 package's own `test/` directory (co-located with the source, the ROS 2 / colcon convention). The proxy pattern keeps test source next to the code it tests while making tests discoverable by `pytest tests/`.
+- **`tests/robot/`** and **`tests/sim/`** — Hermetic **unit** tests (`@pytest.mark.unit`). These are **thin proxy files** that call ``register_unit_tests()`` to register tests from each package's co-located `test/` directory so `pytest tests/` discovers them.
 
 ### Unit tests vs system tests
 

.env

@@ -51,11 +51,6 @@ URDF_FILE="robot_descriptions/iris/urdf/iris_with_sensors.pegasus.robot.urdf"
 
 DEBUG_RVIZ="false"  # "true" or "false". If true, launches RViz alongside the robot via desktop_bringup/robot.launch.xml.
 
-LAUNCH_NATNET="false"
-# PX4 SITL param profile for isaac-sim (simulation/isaac-sim/docker/px4-profiles/).
-# Use "vision" with LAUNCH_NATNET + MAVROS vision_pose; "default" for plain Pegasus SITL.
-PX4_PARAM_PROFILE="default"
-
 # offboard API streaming out. this is so that ports don't conflict for multi-agent FCU communication. 
 OFFBOARD_BASE_PORT=14540
 ONBOARD_BASE_PORT=14580

AGENTS.md

@@ -197,15 +197,15 @@ docker exec airstack-robot-desktop-1 bash -c "ros2 topic echo <topic_name> --onc
    - Verify module behavior in isolation
    - Test with synthetic data
    - Located in module's `test/` directory
-   - **Run in the robot container** with `colcon test` (after `bws`), not via `airstack test -m unit`. The root [`tests/`](tests/) suite does **not** register a `unit` pytest mark; `airstack test -m <mark>` only selects marks declared in [`tests/pytest.ini`](tests/pytest.ini) (`unit`, `build_docker`, `build_packages`, `liveliness`, `sensors`, `takeoff_hover_land`, `integration`, `natnet`).
+   - **Run in the robot container** with `colcon test` (after `bws`), not via `airstack test -m unit`. The root [`tests/`](tests/) suite does **not** register a `unit` pytest mark; `airstack test -m <mark>` only selects marks declared in [`tests/pytest.ini`](tests/pytest.ini) (`unit`, `build_docker`, `build_packages`, `integration`, `liveliness`, `sensors`, `takeoff_hover_land`).
 
    ```bash
    docker exec airstack-robot-desktop-1 bash -c "sws && colcon test --packages-select natnet_ros2 --event-handlers console_direct+"
    ```
 
-2. **Unit tests (`pytest`, `unit` mark):** Fast, hermetic checks. Test **source** lives co-located with each ROS 2 package in `<package>/test/` (standard colcon convention). Thin **proxy** files in [`tests/robot/`](tests/robot/) and [`tests/sim/`](tests/sim/) re-export those tests so `pytest tests/` discovers them. Unit tests run as part of the `system-tests.yml` suite. Example: `airstack test -m unit -v`. See `add-unit-tests` skill.
+2. **Unit tests (`pytest`, `unit` mark):** Fast, hermetic checks. Test **source** lives co-located with each ROS 2 package in `<package>/test/` (standard colcon convention). Thin **proxy** files in [`tests/robot/`](tests/robot/) and [`tests/sim/`](tests/sim/) call `register_unit_tests()` so `pytest tests/` discovers them. Unit tests run as part of the `system-tests.yml` suite. Example: `airstack test -m unit -v`. See `add-unit-tests` skill.
 
-3. **Integration (`pytest`, `integration` mark, [`tests/integration/`](tests/integration/)):** Wire a few real components together — the robot autonomy container plus a host-side component — **without** a sim or GPU. The shared `robot_autonomy_stack` fixture reuses a running `robot-desktop` container or brings one up only when `--run-integration` is passed (else skips), so a plain `pytest tests/` never spins up Docker. First resident: `tests/integration/natnet/` (host NatNet emulator → `natnet_ros2` pose Hz; marks `integration`, `natnet`). Example: `airstack test -m integration --run-integration -v`.
+3. **Integration (`pytest`, `integration` mark, [`tests/integration/`](tests/integration/)):** Wire a few real components together — the robot autonomy container plus a host-side component — **without** a sim or GPU. The shared `robot_autonomy_stack` fixture reuses a running `robot-desktop` container or brings one up only when `--run-integration` is passed (else skips), so a plain `pytest tests/` never spins up Docker. Collection order runs integration after `build_docker` and `build_packages`. First resident: `tests/integration/natnet/` (host NatNet emulator → `natnet_ros2` pose Hz). Example: `airstack test -m integration --run-integration -v`.
 
 4. **System Level (`tests/system/`):** Full simulation tests (Isaac Sim or Microsoft AirSim legacy)
    - End-to-end autonomy stack testing

docs/development/intermediate/testing/index.md

@@ -12,8 +12,8 @@ hardware requirement:
 ## Unit tests (`pytest -m unit`)
 
 Fast, hermetic Python tests that run in seconds with no Docker or GPU. Test source
-lives **co-located with its ROS 2 package** (`<package>/test/`) and is re-exported
-through thin proxy files in `tests/robot/` for centralized discovery.
+lives **co-located with its ROS 2 package** (`<package>/test/`) and is registered
+through thin proxy files in `tests/robot/` via `register_unit_tests()`.
 
 ```bash
 airstack test -m unit -v

docs/development/intermediate/testing/unit_testing.md

@@ -5,7 +5,7 @@ AirStack unit tests are **fast, hermetic, and purely Python** — no Docker stac
 ## Design principles
 
 - **Co-located with source.** Test files live in `<package>/test/` alongside the code they test. This is the standard ROS 2 / colcon convention and ensures tests are discovered by both `colcon test` and `pytest`.
-- **Proxy for centralized discovery.** A thin shim in `tests/robot/<layer>/<package>/` re-exports the test functions so `pytest tests/` (the CI command) and `airstack test -m unit` discover them without any changes to the CI workflow.
+- **Proxy for centralized discovery.** A thin shim in `tests/robot/<layer>/<package>/` calls `register_unit_tests()` (in `tests/conftest.py`) to register co-located `test_*` functions so `pytest tests/` and `airstack test -m unit` discover them. See the `add-unit-tests` skill for the full contract.
 - **`@pytest.mark.unit` on every test.** The `unit` mark is the filter that keeps unit tests isolated from system tests that need Docker, GPUs, and sim licenses.
 
 ## Repository layout

docs/simulation/isaac_sim/docker.md

@@ -12,6 +12,9 @@ simulation/isaac-sim/docker/
 ├── fastdds.xml                   # DDS configuration
 ├── omni_pass.env                 # Omniverse credentials (git-ignored)
 ├── omni_pass_TEMPLATE.env        # Template for credentials
+├── sitl-files/                   # SITL env bundles (PX4_PARAM_* via compose)
+│   ├── default.env               # Plain Pegasus SITL (no param overrides)
+│   └── px4-vision.env            # External vision / NatNet → MAVROS → EKF2
 ├── omniverse.toml                # Omniverse settings
 ├── user.config.json              # Isaac Sim configuration (enables extensions)
 └── user_TEMPLATE.config.json     # Template configuration
@@ -116,11 +119,14 @@ Key variables for Isaac Sim configuration:
 | `ISAAC_SIM_SCRIPT_NAME` | Standalone script filename | - |
 | `PX4_PHYSICS_HZ` | Physics step rate for PX4 SITL — also sets PX4 `IMU_INTEG_RATE` | `250` |
 | `PX4_RENDERING_HZ` | Rendering frame rate for PX4 profiles (independent of physics) | `60` |
+| `SITL_PARAM_PROFILE` | SITL env bundle stem under `sitl-files/` (e.g. `default`, `px4-vision`) | `default` |
 | `ARDUPILOT_PHYSICS_HZ` | Physics step rate for ArduPilot SITL | `800` |
 | `ARDUPILOT_RENDERING_HZ` | Rendering frame rate for ArduPilot profiles | `120` |
 
 `PX4_PHYSICS_HZ` and `PX4_RENDERING_HZ` are set in the top-level `.env`. Pegasus defaults to 250 Hz but AirStack runs PX4 at **100 Hz** for near-real-time performance. See [Pegasus Scene Setup → Physics Rate](pegasus_scene_setup.md) for valid values and the full configuration flow.
 
+SITL parameter bundles live in `sitl-files/` (`default.env`, `px4-vision.env`). Compose loads `sitl-files/${SITL_PARAM_PROFILE}.env` into the container; see [Pegasus Scene Setup → PX4 SITL env bundles](pegasus_scene_setup.md#px4-sitl-env-bundles-external-vision).
+
 **Example overrides:**
 
 ```bash

docs/simulation/isaac_sim/pegasus_scene_setup.md

@@ -165,19 +165,19 @@ PX4_RENDERING_HZ="30"
 - **`PX4_PHYSICS_HZ`** — Sets `physics_dt = 1 / PX4_PHYSICS_HZ` in Isaac Sim's physics scene, and automatically syncs PX4's `IMU_INTEG_RATE` parameter to the same value via `PX4LaunchTool` → `px4-rc.simulator`. Patched within the Docker image to read the environment variable and set the IMU_INTEG_RATE parameter.
 - **`PX4_RENDERING_HZ`** — Sets the rendering frame rate independently of physics. 30 Hz rendering has no effect on physics accuracy or PX4 behavior, but does slightly affect performance due to resource usage.
 
-### PX4 parameter profiles (external vision)
+### PX4 SITL env bundles (external vision)
 
-Isaac Sim loads a layered PX4 SITL parameter profile from `simulation/isaac-sim/docker/px4-profiles/`. Select the profile with `PX4_PARAM_PROFILE` in the top-level `.env`:
+Isaac Sim loads optional PX4 SITL parameter bundles from `simulation/isaac-sim/docker/sitl-files/`. Select the bundle with `SITL_PARAM_PROFILE` in the top-level `.env` (compose maps this to `sitl-files/<profile>.env`):
 
 ```bash
-# Plain Pegasus SITL
-PX4_PARAM_PROFILE="default"
+# Plain Pegasus SITL (default.env — comments only, no PX4_PARAM_* overrides)
+SITL_PARAM_PROFILE="default"
 
-# NatNet / mocap → MAVROS vision_pose → EKF2 external vision
-PX4_PARAM_PROFILE="vision"
+# NatNet / mocap → MAVROS vision_pose → EKF2 external vision (px4-vision.env)
+SITL_PARAM_PROFILE="px4-vision"
 ```
 
-Profiles inject `PX4_PARAM_*` variables into the isaac-sim container; PX4 applies them at boot via `init.d-posix/rcS`. Pair `vision` with robot-side `LAUNCH_NATNET=true` and `publish_to_mavros: true` in `natnet_config.yaml`.
+Bundles inject `PX4_PARAM_*` variables into the isaac-sim container; PX4 applies them at boot via `init.d-posix/rcS`. Pair `px4-vision` with robot-side `LAUNCH_NATNET=true` and `publish_to_mavros: true` in `natnet_config.yaml`.
 
 Convenience bundle: `airstack up --env-file overrides/isaac-natnet-vision.env`.
 

overrides/isaac-natnet-vision.env

@@ -2,6 +2,6 @@
 # Usage: airstack up --env-file overrides/isaac-natnet-vision.env
 
 LAUNCH_NATNET=true
-PX4_PARAM_PROFILE=vision
+SITL_PARAM_PROFILE=px4-vision
 ISAAC_SIM_USE_STANDALONE=true
 ISAAC_SIM_SCRIPT_NAME=example_multi_px4_pegasus_launch_script.py