Real-robot deployment fixes: make RECORD_BAGS and ROBOT_NAME overrides take effect - #377
Open
JohnYanxinLiu wants to merge 1 commit into
Open
Real-robot deployment fixes: make RECORD_BAGS and ROBOT_NAME overrides take effect#377JohnYanxinLiu wants to merge 1 commit into
JohnYanxinLiu wants to merge 1 commit into
Conversation
…effect 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two deployment knobs that look wired up but are inert. Both are forwarded into the robot container and then read by nobody, and neither failure is visible until you go looking for output that never appeared. Found while porting fixes back from a real mocap-room drone deployment.
Independent of the OptiTrack PR stack (#374/#375/#376) — nothing here depends on it, and it can merge in either order.
RECORD_BAGSnever enabled bag recording on a robotrobot-base-docker-compose.yamlhas forwarded the variable since #318, but:logging_bringup/launch/logging.launch.xmlhardcoded<arg name="record_bag" default="false" />onboard_autonomy_all.launch.xml:56includes that file with no argumentsso nothing ever overrode the default. Only
gcs.launch.xmlreads$(env RECORD_BAGS)— which is why the flag appears to work if you only check the GCS.Fixing the gate alone wasn't enough:
bag_record_nodecomes up withself.active = Falseand the only path torun()is theset_recording_statussubscription, so you'd get a running recorder that records nothing on an unattended robot. Added anauto_startparameter (declaredFalse, so existing toggle-driven behavior is unchanged) which the launch file sets fromBAG_AUTO_START, defaulttrue—RECORD_BAGSalready gates whether the node runs at all.Also added
LOG_CONFIGto select which topic set inlogging_bringup/configto record, defaulting to the currentlog.yaml.ROBOT_NAME/ROS_DOMAIN_IDcouldn't be overridden#370 made
.bashrckeep a pre-setROBOT_NAMEinstead of always overwriting it from the container/hostname map — but no compose service declares the variable, and compose only injects what a service names inenvironment:. So setting it in an override.envdid nothing. This is the missing half.It matters on hosts whose hostname misses the
.*robot-.*(\d+)rule indefault_robot_name_map.yaml: a Jetson namedairlab-desktopfalls through to the catch-all, comes up asunknown_roboton domain 0, and is silently invisible to the rest of the stack.Safe when unset. Compose passes an empty string;
.bashrc's[ -z "${ROBOT_NAME:-}" ]and[ -z "$existing_robot_domain_id" ]guards treat that as absent, so the existing mapping path is untouched.robot-offboard's explicitROS_DOMAIN_ID=0also still wins, since a service's ownenvironment:overrides what it extends.Single-robot only, documented inline: 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.Behavior change to be aware of
Anyone currently running
RECORD_BAGS=truefor GCS bags will now also get a recorder on every robot, actively recording. That's the intended semantics, but it isn't a no-op for existing users —RECORD_BAGS=falseorBAG_AUTO_START=falseopts out.Testing
logging.launch.xmlparses;robot-base-docker-compose.yamlparses;bag_record_node.pyparses.ROBOT_NAMEhalf is what the real Jetson deployment ran with.🤖 Generated with Claude Code