From 4cddd73f1bd3410bd875786638d401f2857ee451 Mon Sep 17 00:00:00 2001 From: nityam Date: Tue, 8 Sep 2026 00:01:59 +0530 Subject: [PATCH 1/3] robotd: a mode switch waits for a policy load, and a policy change waits for a shutdown --- robotd/src/main.rs | 130 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+) diff --git a/robotd/src/main.rs b/robotd/src/main.rs index 0c4eca56..4ca343f1 100644 --- a/robotd/src/main.rs +++ b/robotd/src/main.rs @@ -2043,6 +2043,16 @@ async fn control_loop( ); } else if mode_change.is_some() { tracing::warn!(mode = target.as_str(), "a mode switch is already in flight"); + } else if pending_swap.is_some() { + // The other half of the guard on the policy change below. A change is derived + // from the params that are running, so a switch accepted now would rebuild + // everything at the home pose and then have the load land on top of it, putting + // the old mode's params, networks and slot report back with `robot.mode` still + // saying the new one. + tracing::warn!( + mode = target.as_str(), + "mode switch refused: a policy change is still loading; ask again when it lands" + ); } else { tracing::warn!( from = policy_params.mode.as_str(), @@ -2081,6 +2091,15 @@ async fn control_loop( if let Some(change) = intents.take_policy_change() { if mode_change.is_some() || pending_swap.is_some() { tracing::warn!("a policy change is already in flight; ignoring this one"); + } else if shutdown_sit.is_some() { + // A change to the network driving takes the mode switch's path home, and from + // inside the sit that stands the robot up at gain until the sit cuts the torque + // out from under it: the shape of #159, by one more door. Nothing about a + // shutdown wants a new network. + tracing::warn!( + change = describe_change(&change), + "policy change refused: the robot is shutting down" + ); } else if let Some(candidate) = candidate_params(&change, &policy_params, ¶ms_path, &mut slot_errors) { @@ -7730,4 +7749,115 @@ mod tests { assert!(Bringup::Limp.homing_target(since).is_none()); assert!(Bringup::Ready.homing_target(since).is_none()); } + + async fn wait_until(what: impl Fn() -> bool, deadline: Duration, or: &str) { + let started = Instant::now(); + while !what() { + assert!(started.elapsed() < deadline, "{or}"); + tokio::time::sleep(Duration::from_millis(2)).await; + } + } + + /// **A mode switch and a policy change cannot both be in flight.** The guard existed one way: + /// a change is refused while a switch is in flight, because the switch is about to rebuild + /// everything the change was derived from. The other way was open. A switch accepted while + /// a load was still on its thread went home, swapped in the other mode's bundle, and then the + /// load landed and put the old mode's params, networks and slot report back, with + /// `robot.mode` still reporting the new one. On a robot a load is most of a second, which is + /// a long time to be holding D-pad up in. + /// + /// With no runtime here the load lands within a tick, so the window is the gap between the + /// thread starting and the loop polling it. Several tries, so that main trips it and the fix + /// never does. The check after each is the invariant itself: `robot.mode` and the walk slot + /// name the same mode. + #[tokio::test] + async fn a_mode_switch_is_not_undone_by_a_policy_load_that_lands_after_it() { + let mut params = Params::default(); + // The load has to land for the bug to show, and there is no runtime here to load with. + params.policy.enabled = false; + let s = Arc::new(RobotState::new( + ¶ms, + std::path::Path::new("/test/robotd.toml"), + false, + false, + )); + let intents = Arc::new(Intents::new()); + let handle = tokio::spawn(control_loop( + FakeIo::at(DEFAULT_POSITION).frozen(), + Arc::clone(&s), + Arc::clone(&intents), + params, + PathBuf::from("/test/robotd.toml"), + Duration::from_millis(2), + noop_poweroff(), + )); + + wait_until( + || s.ticks.load(Ordering::Relaxed) >= 5, + Duration::from_secs(2), + "no ticks", + ) + .await; + intents.request_init(); + wait_until( + || s.homed.load(Ordering::Relaxed), + HOME_RAMP + Duration::from_secs(2), + "init never reached home", + ) + .await; + + let walk_slot = |s: &RobotState| { + s.policy_slots + .load() + .iter() + .find(|slot| slot.slot == "walk") + .and_then(|slot| slot.path.clone()) + .unwrap_or_default() + }; + for attempt in 0..5 { + let mode = mode_of(s.mode.load(Ordering::Relaxed)); + let target = if mode == Mode::Roller { + Mode::Walk + } else { + Mode::Roller + }; + + intents.request_policy_change(intents::PolicyChange::Slot { + slot: params::Slot::KickLeft, + path: None, + }); + // Taken on the next tick, which starts its load. The switch lands on the tick after. + let at = s.ticks.load(Ordering::Relaxed); + wait_until( + || s.ticks.load(Ordering::Relaxed) > at, + Duration::from_secs(2), + "stalled", + ) + .await; + intents.request_mode_switch(mode_code(target)); + + // A switch that was accepted starts a ramp; give it and the load time to finish. + let at = s.ticks.load(Ordering::Relaxed); + wait_until( + || s.ticks.load(Ordering::Relaxed) >= at + 10, + Duration::from_secs(2), + "stalled", + ) + .await; + if !s.homed.load(Ordering::Relaxed) { + tokio::time::sleep(HOME_RAMP + Duration::from_millis(300)).await; + } + + let mode = mode_of(s.mode.load(Ordering::Relaxed)); + let walk = walk_slot(&s); + assert_eq!( + mode == Mode::Roller, + walk.ends_with("roller.onnx"), + "attempt {attempt}: robot.mode says {mode:?} and the walk slot is {walk}" + ); + } + + s.shutdown.store(true, Ordering::Relaxed); + handle.await.unwrap(); + } } From 330affa018194111da2f2cd41eaab8fcc271ddcc Mon Sep 17 00:00:00 2001 From: nityam Date: Thu, 10 Sep 2026 16:24:59 +0530 Subject: [PATCH 2/3] robotd: a mode switch waits for a shutdown too --- robotd/src/main.rs | 63 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/robotd/src/main.rs b/robotd/src/main.rs index 040a5409..acc66364 100644 --- a/robotd/src/main.rs +++ b/robotd/src/main.rs @@ -2214,6 +2214,15 @@ async fn control_loop( mode = target.as_str(), "mode switch refused: a policy change is still loading; ask again when it lands" ); + } else if shutdown_sit.is_some() || powered_off { + // The robot is on its way down. Homing from inside the sit would stand it back + // up at gain, and the sit would then cut the torque out from under it: the shape + // of #159, by one more door after `robot.init` and the enable-driven bring-up, + // both of which 7563b4c already gates on `powered_off`. + tracing::warn!( + mode = target.as_str(), + "mode switch refused: the robot is shutting down" + ); } else { tracing::warn!( from = policy_params.mode.as_str(), @@ -8259,6 +8268,60 @@ mod tests { handle.await.unwrap(); } + /// A robot on its way down is not switched. Homing from inside the sit would stand it back up + /// at gain, and then the sit would cut the torque out from under it — #159 by one more door, + /// after `robot.init` and the enable-driven bring-up, which 7563b4c already gates. + /// + /// The sit itself needs the sitstand network, so this drives the sibling path CI can reach: + /// no policy, so `robot.shutdown` powers off at once. The gate is the same one. + #[tokio::test] + async fn a_mode_switch_after_power_off_is_refused() { + let io = FakeIo::at(DEFAULT_POSITION).frozen(); + let s = Arc::new(state()); + let intents = Arc::new(Intents::new()); + let handle = tokio::spawn({ + let s = Arc::clone(&s); + let intents = Arc::clone(&intents); + async move { + let mut io = io; + control_loop_probe_with(&mut io, s, intents, Duration::from_millis(2)).await; + } + }); + wait_until( + || s.ticks.load(Ordering::Relaxed) >= 5, + Duration::from_secs(2), + "no ticks", + ) + .await; + intents.request_init(); + wait_until( + || s.homed.load(Ordering::Relaxed), + HOME_RAMP + Duration::from_secs(2), + "init never reached home", + ) + .await; + + intents.request_shutdown(); + let at = s.ticks.load(Ordering::Relaxed); + wait_until( + || s.ticks.load(Ordering::Relaxed) >= at + 3, + Duration::from_secs(2), + "stalled", + ) + .await; + + intents.request_mode_switch(mode_code(Mode::Roller)); + tokio::time::sleep(HOME_RAMP + Duration::from_millis(500)).await; + assert_eq!( + mode_of(s.mode.load(Ordering::Relaxed)), + Mode::Walk, + "a mode switch went through on a robot that was powering off" + ); + + s.shutdown.store(true, Ordering::Relaxed); + handle.await.unwrap(); + } + /// `1e400` on the wire parses as infinity. Folded into the EMA it is permanent — nothing /// finite climbs back out — and with the safety layer refusing non-finite targets, one bad /// `robot.move` would freeze the robot on its hold pose until reboot. Dropped instead. From 51319e078112d4ab1e3ac8b2b69a489ae4636053 Mon Sep 17 00:00:00 2001 From: nityam Date: Thu, 10 Sep 2026 16:37:39 +0530 Subject: [PATCH 3/3] Revert "robotd: a mode switch waits for a shutdown too" This reverts commit 330affa018194111da2f2cd41eaab8fcc271ddcc. --- robotd/src/main.rs | 63 ---------------------------------------------- 1 file changed, 63 deletions(-) diff --git a/robotd/src/main.rs b/robotd/src/main.rs index acc66364..040a5409 100644 --- a/robotd/src/main.rs +++ b/robotd/src/main.rs @@ -2214,15 +2214,6 @@ async fn control_loop( mode = target.as_str(), "mode switch refused: a policy change is still loading; ask again when it lands" ); - } else if shutdown_sit.is_some() || powered_off { - // The robot is on its way down. Homing from inside the sit would stand it back - // up at gain, and the sit would then cut the torque out from under it: the shape - // of #159, by one more door after `robot.init` and the enable-driven bring-up, - // both of which 7563b4c already gates on `powered_off`. - tracing::warn!( - mode = target.as_str(), - "mode switch refused: the robot is shutting down" - ); } else { tracing::warn!( from = policy_params.mode.as_str(), @@ -8268,60 +8259,6 @@ mod tests { handle.await.unwrap(); } - /// A robot on its way down is not switched. Homing from inside the sit would stand it back up - /// at gain, and then the sit would cut the torque out from under it — #159 by one more door, - /// after `robot.init` and the enable-driven bring-up, which 7563b4c already gates. - /// - /// The sit itself needs the sitstand network, so this drives the sibling path CI can reach: - /// no policy, so `robot.shutdown` powers off at once. The gate is the same one. - #[tokio::test] - async fn a_mode_switch_after_power_off_is_refused() { - let io = FakeIo::at(DEFAULT_POSITION).frozen(); - let s = Arc::new(state()); - let intents = Arc::new(Intents::new()); - let handle = tokio::spawn({ - let s = Arc::clone(&s); - let intents = Arc::clone(&intents); - async move { - let mut io = io; - control_loop_probe_with(&mut io, s, intents, Duration::from_millis(2)).await; - } - }); - wait_until( - || s.ticks.load(Ordering::Relaxed) >= 5, - Duration::from_secs(2), - "no ticks", - ) - .await; - intents.request_init(); - wait_until( - || s.homed.load(Ordering::Relaxed), - HOME_RAMP + Duration::from_secs(2), - "init never reached home", - ) - .await; - - intents.request_shutdown(); - let at = s.ticks.load(Ordering::Relaxed); - wait_until( - || s.ticks.load(Ordering::Relaxed) >= at + 3, - Duration::from_secs(2), - "stalled", - ) - .await; - - intents.request_mode_switch(mode_code(Mode::Roller)); - tokio::time::sleep(HOME_RAMP + Duration::from_millis(500)).await; - assert_eq!( - mode_of(s.mode.load(Ordering::Relaxed)), - Mode::Walk, - "a mode switch went through on a robot that was powering off" - ); - - s.shutdown.store(true, Ordering::Relaxed); - handle.await.unwrap(); - } - /// `1e400` on the wire parses as infinity. Folded into the EMA it is permanent — nothing /// finite climbs back out — and with the safety layer refusing non-finite targets, one bad /// `robot.move` would freeze the robot on its hold pose until reboot. Dropped instead.