From 5710c4b2c2f0c2128273088b0d519138e90f31b2 Mon Sep 17 00:00:00 2001 From: John Date: Fri, 31 Jul 2026 18:39:55 -0400 Subject: [PATCH] fix(deploy): make RECORD_BAGS and ROBOT_NAME overrides actually take effect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two deployment knobs that looked wired up but were inert. Both are forwarded into the container and then dropped on the floor; neither failure is visible until you go looking for the output that never appeared. RECORD_BAGS: robot-base-docker-compose.yaml has forwarded it since #318, but logging.launch.xml hardcoded `record_bag=false` and onboard_autonomy_all.launch.xml includes that file with no arguments — so nothing ever overrode the default. Only gcs.launch.xml read the variable, which is why it appears to work if you check the GCS. logging.launch.xml now reads it, and gains LOG_CONFIG to select a topic set. The recorder also came up idle: `self.active = False` with the only path to run() being the set_recording_status subscription. Enabling the gate alone would have produced a running node recording nothing, so bag_record_node gains an auto_start parameter (declared False, so the toggle-driven behavior is unchanged for existing users) and the launch file sets it from BAG_AUTO_START, default true — RECORD_BAGS already gates whether the node runs at all. ROBOT_NAME / ROS_DOMAIN_ID: #370 made .bashrc keep a pre-set ROBOT_NAME instead of always overwriting it from the container/hostname map, but no compose service declares the variable, so setting it in an override .env did nothing. This is the missing half. It matters on hosts whose hostname misses the '.*robot-.*(\d+)' rule — a Jetson named "airlab-desktop" falls through to the catch-all and comes up as unknown_robot on domain 0, silently invisible to the rest of the stack. Safe when unset: compose passes an empty string, and .bashrc's `[ -z ... ]` guards treat that as absent, so the existing mapping path is untouched. Documented inline as single-robot only — replica deployments share one env and would collapse every replica onto the same name and domain. Co-Authored-By: Claude Opus 5 --- .env | 2 +- CHANGELOG.md | 2 ++ .../bag_record_pid/bag_record_node.py | 12 ++++++++++- .../logging_bringup/launch/logging.launch.xml | 21 +++++++++++++++---- robot/docker/robot-base-docker-compose.yaml | 18 +++++++++++++++- 5 files changed, 48 insertions(+), 7 deletions(-) diff --git a/.env b/.env index 00cd4c639..04de90a2d 100644 --- a/.env +++ b/.env @@ -12,7 +12,7 @@ PROJECT_NAME="airstack" # If you've run ./airstack.sh setup, then this will auto-generate from the git commit hash every time a change is made # to a Dockerfile or docker-compose.yaml file. Otherwise this can also be set explicitly to make a release version. # auto-generated from git commit hash -VERSION="0.19.0-alpha.8" +VERSION="0.19.0-alpha.12" # Choose "dev" or "prebuilt". "dev" is for mounted code that must be built live. "prebuilt" is for built ros_ws baked into the image DOCKER_IMAGE_BUILD_MODE="dev" # Where to push and pull images from. Can replace with your docker hub username if using docker hub. diff --git a/CHANGELOG.md b/CHANGELOG.md index a5ea32f38..9fe8acf06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Robot name resolution now honors a pre-set `ROBOT_NAME` (e.g. injected via docker compose) instead of always overriding it from the container/hostname mapping (`robot/docker/.bashrc`) - Robot name-map catch-all fallback now maps to `unknown_robot` (valid ROS namespace token) instead of `unknown-robot` (`default_robot_name_map.yaml`) - l4t robot image: replace dustynv's `/ros_entrypoint.sh` with a passthrough so its prebuilt source-ROS libs (older `fastcdr`) no longer shadow the apt Jazzy runtime and crash apt-built nodes like MAVROS +- `RECORD_BAGS=true` never enabled bag recording on a robot: `logging.launch.xml` hardcoded `record_bag=false` and `onboard_autonomy_all.launch.xml` includes it with no arguments, so the variable was forwarded into the container and read by nobody (only `gcs.launch.xml` consumed it). It now reads `RECORD_BAGS`, selects its topic set via `LOG_CONFIG`, and starts recording at bringup (`auto_start`, `BAG_AUTO_START=false` to opt out) +- `ROBOT_NAME` / `ROS_DOMAIN_ID` can now be set from an override env file: `.bashrc` already honored a pre-set `ROBOT_NAME`, but no compose service forwarded it, so the override had no effect. Required for hosts whose hostname doesn't match `default_robot_name_map.yaml` (a Jetson named `airlab-desktop` otherwise comes up as `unknown_robot` on domain 0). Single-robot only — replicas must leave these unset ## [1.0.0] - 2024-12-19 diff --git a/common/ros_packages/logging/bag_recorder_pid/bag_record_pid/bag_record_node.py b/common/ros_packages/logging/bag_recorder_pid/bag_record_pid/bag_record_node.py index d3d746d99..363a5d781 100644 --- a/common/ros_packages/logging/bag_recorder_pid/bag_record_pid/bag_record_node.py +++ b/common/ros_packages/logging/bag_recorder_pid/bag_record_pid/bag_record_node.py @@ -29,10 +29,16 @@ def __init__(self): self.declare_parameter( "mcap_qos_dir", "" ) - + + # Begin recording immediately on bringup instead of waiting for a + # set_recording_status=true message. Defaults false so the existing + # toggle-driven behavior is unchanged for anyone not setting it. + self.declare_parameter("auto_start", False) + self.cfg_path = self.get_parameter("cfg_path").get_parameter_value().string_value self.output_dir = self.get_parameter("output_dir").get_parameter_value().string_value self.mcap_qos_dir = self.get_parameter("mcap_qos_dir").get_parameter_value().string_value + self.auto_start = self.get_parameter("auto_start").get_parameter_value().bool_value self.active = False self.cfg = yaml.safe_load(open(self.cfg_path)) @@ -71,6 +77,10 @@ def __init__(self): self.timer = self.create_timer(0.5, self.pub_status_callback) + if self.auto_start: + self.get_logger().info("auto_start=true — beginning recording on bringup.") + self.run() + def add_topics(self): '''The configuration file looks like diff --git a/common/ros_packages/logging/logging_bringup/launch/logging.launch.xml b/common/ros_packages/logging/logging_bringup/launch/logging.launch.xml index 6c6df0502..72d89c98e 100644 --- a/common/ros_packages/logging/logging_bringup/launch/logging.launch.xml +++ b/common/ros_packages/logging/logging_bringup/launch/logging.launch.xml @@ -1,13 +1,26 @@ - - + + + + + - + + + - + diff --git a/robot/docker/robot-base-docker-compose.yaml b/robot/docker/robot-base-docker-compose.yaml index a4ed7e1bb..db986660b 100644 --- a/robot/docker/robot-base-docker-compose.yaml +++ b/robot/docker/robot-base-docker-compose.yaml @@ -10,8 +10,24 @@ services: - DISPLAY=${DISPLAY} - QT_X11_NO_MITSHM=1 - QT_QPA_PLATFORM - # Record bags + # Record bags. LOG_CONFIG picks which topic set in logging_bringup/config to + # record; BAG_AUTO_START=false reverts to waiting for a set_recording_status + # toggle instead of recording from bringup. Both read by logging.launch.xml. - RECORD_BAGS=${RECORD_BAGS} + - LOG_CONFIG=${LOG_CONFIG:-log.yaml} + - BAG_AUTO_START=${BAG_AUTO_START:-true} + # Pre-set robot identity. .bashrc keeps a non-empty ROBOT_NAME instead of + # resolving one from the container/hostname map, so this is the only way to name + # a robot whose hostname doesn't match default_robot_name_map.yaml (e.g. a Jetson + # named "airlab-desktop", which otherwise falls through to unknown_robot on + # domain 0). Unset yields an empty string, which .bashrc treats as absent — so + # the existing mapping path is unchanged when an override doesn't set these. + # + # SINGLE ROBOT ONLY: replica deployments (deploy.replicas / NUM_ROBOTS>1) share + # one env, so setting these would collapse every replica onto the same name and + # domain. Leave them unset there and let the container-name mapping do its job. + - ROBOT_NAME=${ROBOT_NAME} + - ROS_DOMAIN_ID=${ROS_DOMAIN_ID} # docker compose interpolation to env variables - AUTONOMY_ROLE=${AUTONOMY_ROLE:-full} - URDF_FILE=${URDF_FILE}