Skip to content

Add fuse odometry localization for the hangar_sim mobile base#790

Open
bkanator wants to merge 5 commits into
mainfrom
feat/19667-fuse-odometry
Open

Add fuse odometry localization for the hangar_sim mobile base#790
bkanator wants to merge 5 commits into
mainfrom
feat/19667-fuse-odometry

Conversation

@bkanator

@bkanator bkanator commented Jul 17, 2026

Copy link
Copy Markdown

[written by AI]

Closes #19667.

Problem

hangar_sim's mobile base drove on raw MuJoCo odometry — too clean to exercise the real localization stack. To represent a real Ridgeback, the base needs fuse-fused odometry (wheel + IMU) with realistic drift, and beluga_amcl must localize reliably against it during Navigate to Clicked Point — including where the scene is degenerate for scan matching (the smooth fuselage, the unmapped picking boxes).

Approach

  • Fuse on by default (use_fuse=true): fuse fuses wheel odometry + IMU into /odom_filtered.
  • odomworld drift injection: with fuse on, odom_world_drift publishes a live odomworld transform from the fuse estimate, so AMCL sees real drift to correct while worldbase stays ground truth for whole-body planning. odom_topic stays on raw /odom — no manual swap.
  • Slip-aware wheel covariance (slip_aware_odom, new C++ node): republishes wheel odometry with a yaw covariance that grows during sustained in-place spin (mecanum rollers slip), so fuse defers to the IMU while spinning and trusts the wheels when driving straight.
  • AMCL tuning for the mecanum base: OmniMotionModel; alpha1 0.1→0.4 to stop yaw lock-loss during spins; update_min_a 0.1 with resample_interval 3 to correct often without particle depletion.
  • AMCL likelihood relaxation: sigma_hit 0.1→0.25, z_hit 0.9→0.65, z_rand 0.1→0.3 so the unmapped picking boxes read as outliers instead of yanking the whole estimate — the box-divergence axis the fuse fix alone didn't touch.
  • Degeneracy gate (amcl_odom_gate, new C++ node): where AMCL's scan-match goes degenerate — the base hugging the smooth fuselage (slide-along ambiguity) or crossing the transient unmapped boxes — it holds the last good mapodom and coasts on fuse's odometry through the zone, then blends back once AMCL is trustworthy again. A large correction is neither hard-accepted nor hard-rejected: it is accepted only if it persists over a sliding window (a real fix persists at one pose; an ambiguity teleport thrashes), gated on both position and yaw, with particle-spread hysteresis as a second, independent trigger — so a valid large correction such as recovering from a bad initial-pose seed still gets through. AMCL runs tf_broadcast:=false; the gate is the sole mapodom publisher. The pure decision logic (detail::updateGate) has no ROS/TF deps and is unit-tested for every case (transparent tracking, thrashing/yaw teleports held, persistent correction accepted, spread hysteresis, SE(2) math).
  • Removed the per-objective SetInitialPose reseed from the clicked-point Objectives: superseded by slip_aware_odom's cross-controller re-anchoring (which keeps odom continuous across the whole-body↔nav handoff it was added for), and the unconditional reseed could cement a drifting estimate.
  • Turn-rate cap: wz_max 0.6 — below the velocity_smoother's 1.0 cap so it actually binds — keeps commanded spins within AMCL's correction bandwidth so the map tracks instead of lagging.
  • CPU reductions: odom_rate 50 Hz, tf_publish_rate 30 Hz, and the broadcaster update_rates decoupled from the high-rate control loop, giving the localization stack headroom.

Results

On a fresh sim with adequate CPU, a 40-goal aggressive-turning route reaches 40/40 goals at ~0.12 m ATE with no divergence, reproducibly across runs. The map still lags slightly during fast turns (inherent AMCL correction latency), but Navigate to Clicked Point reaches every goal.

The degeneracy gate targets the two failure modes that remained: on a stress test that drives a goal 0.3 m from the smooth fuselage (deep in costmap inflation), bare likelihood_field diverges and never recovers on ~75% of attempts; with the gate that drops to ~10% transient failures that self-recover within 1–2 attempts. The box cluster is contained with no strand. The gate assumes odometry is trustworthy for the (transient) duration of a degenerate zone — a bound documented in the node.

Localization quality is CPU-bound: a co-scheduled second sim can starve AMCL into divergence even with this config, so run one sim per host when benchmarking.

Docs

Paired documentation PR: PickNikRobotics/moveit_pro#20578 (Localization Tuning + Whole-Body Mobile Architecture guides).

Manual verification

ros2 launch hangar_sim and run Navigate to Clicked Point to a goal that requires a large heading change; the robot reaches the clicked point. use_fuse:=false falls back to raw odometry.

Release notes

Enhancement: hangar_sim now localizes its mobile base with fuse odometry (wheel + IMU) and tuned beluga_amcl, with a degeneracy-aware gate that keeps localization stable where the scan match is ambiguous (the smooth fuselage and unmapped boxes), so Navigate to Clicked Point reaches goals reliably during simulated navigation.

@bkanator bkanator added this to the 10.0.0 milestone Jul 17, 2026
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Changes

Adds slip-aware odometry forwarding, dynamic odom-to-world drift correction, and AMCL map-to-odom gating. Fuse is enabled by default, new nodes are wired into simulation launch, localization parameters are retuned, and navigation trees no longer reset the initial pose.

Fuse localization integration

Layer / File(s) Summary
Ground-truth drift publisher
src/hangar_sim/description/ur5e_ridgeback.xml, src/hangar_sim/script/odom_world_drift.py, src/hangar_sim/CMakeLists.txt
Adds a MuJoCo ground-truth site and publishes dynamic odom-to-world drift from fused odometry and virtual-rail joint state.
Slip-aware odometry relay
src/hangar_sim/src/slip_aware_odom.cpp, src/hangar_sim/config/fuse/fuse.yaml
Relays controller odometry continuously across source changes, applies yaw covariance during slip, holds stale output, and routes fuse through /odom_slip_aware.
AMCL odometry gate and tests
src/hangar_sim/include/hangar_sim/amcl_odom_gate_logic.hpp, src/hangar_sim/src/amcl_odom_gate*.cpp, src/hangar_sim/test/test_amcl_odom_gate.cpp, src/hangar_sim/CMakeLists.txt
Adds planar gating logic with spread hysteresis and persistent correction acceptance, integrates it with AMCL particle clouds and TF, and adds unit coverage.
Fuse launch and localization configuration
src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py, src/hangar_sim/params/nav2_params.yaml, src/hangar_sim/config/control/*, src/hangar_sim/description/picknik_ur_mujoco_ros2_control.xacro, src/hangar_sim/package.xml, src/hangar_sim/objectives/*.xml
Enables fuse by default, conditionally launches odometry and gating nodes, adjusts publish rates and localization tuning, adds dependencies, and removes initial-pose actions from navigation trees.

Possibly related issues

  • PickNikRobotics/moveit_pro issue 19667 — Covers fuse/AMCL integration and navigation localization tuning implemented by this change.

Possibly related PRs

Suggested reviewers: griswaldbrooks


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore (reviewers only)

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Human Review Check ❌ Error FAIL: it adds a new public header/API and new executables, plus default-fuse launch/config changes across sim, params, and tests—too broad for low-risk. This PR requires review by a requested human reviewer. After review, a non-author requested reviewer should override this pre-merge check.
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly matches the localization, odometry, AMCL, launch, and tuning changes in the patch.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@bkanator
bkanator force-pushed the feat/19667-fuse-odometry branch from 236f955 to 99cb7f5 Compare July 17, 2026 18:18
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@bkanator
bkanator force-pushed the feat/19667-fuse-odometry branch from 99cb7f5 to ecb0e20 Compare July 17, 2026 18:41
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@bkanator
bkanator force-pushed the feat/19667-fuse-odometry branch from ecb0e20 to 98ef35f Compare July 19, 2026 15:34
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@bkanator
bkanator marked this pull request as ready for review July 21, 2026 11:47

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py`:
- Around line 319-335: Update the odom_world_drift and slip_aware_odom Node
definitions to pass the use_sim_time LaunchConfiguration as their ROS parameter,
ensuring both nodes use simulation time consistently when stamping outputs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6834a0a6-300e-4af5-bd44-d25a11fda468

📥 Commits

Reviewing files that changed from the base of the PR and between 6cfc8c0 and 98ef35f.

📒 Files selected for processing (10)
  • src/hangar_sim/CMakeLists.txt
  • src/hangar_sim/config/control/picknik_ur.ros2_control.yaml
  • src/hangar_sim/config/fuse/fuse.yaml
  • src/hangar_sim/description/picknik_ur_mujoco_ros2_control.xacro
  • src/hangar_sim/description/ur5e_ridgeback.xml
  • src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py
  • src/hangar_sim/package.xml
  • src/hangar_sim/params/nav2_params.yaml
  • src/hangar_sim/script/odom_world_drift.py
  • src/hangar_sim/src/slip_aware_odom.cpp

Comment thread src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py
…667)

Localize the Ridgeback mobile base with fuse (wheel + IMU) odometry and tuned
beluga_amcl so Navigate to Clicked Point reaches goals during simulated navigation.

- use_fuse defaults to true; fuse fuses wheel odometry + IMU into /odom_filtered.
- odom_world_drift publishes a live odom->world transform from the fuse estimate so
  AMCL corrects real drift while world->base stays ground truth for whole-body planning.
- slip_aware_odom (new node) forwards the active platform controller's odometry to fuse
  with a spin-aware yaw covariance, stitches the nav2 <-> whole-body controller handoff
  into one continuous stream, and holds the last pose across the brief switch gap so fuse
  never coasts (which otherwise jumped the estimate and flipped the map in whole-body).
- AMCL tuned for the mecanum base: OmniMotionModel, alpha1 0.4, update_min_a 0.1 with
  resample_interval 3.
- MPPI wz_max capped at 0.6 (below the velocity_smoother cap so it binds) to keep
  commanded spins within AMCL's correction bandwidth.
- odom/tf and broadcaster rates decoupled from the control loop for CPU headroom.
- base_gt MuJoCo site publishes ground-truth base TF for localization QA.

Closes #19667.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@bkanator
bkanator force-pushed the feat/19667-fuse-odometry branch from 98ef35f to b3952a6 Compare July 21, 2026 12:59
@bkanator

Copy link
Copy Markdown
Author

[written by AI]

Pre-merge reviewer gate

Ran code-reviewer, documentation-bot, platform-architect-bot, roboticist-bot, and the hangar_sim integration test.

Applied

  • odom_world_drift.py: re-resolve the /joint_states rail-joint indices whenever the name list changes — guards a stale-index IndexError on the multi-publisher topic.
  • slip_aware_odom.cpp: hold-timer stamp now advances the last relayed (controller-clock) stamp instead of node now(), so held samples stay on the source clock and strictly increase; [[nodiscard]] on the SE(2) helpers; ROS entities declared last (teardown order); comments pinning the mutually-exclusive-callback-group (lock-free) invariant, the real ~500 Hz source rate (the forked controller ignores publish_rate: 50.0), and the differential-only meaning of the published pose.
  • launch: use_sim_time now plumbed to slip_aware_odom and odom_world_drift, matching their siblings.
  • docs (#20578): corrected the odom → world transform-tree entry (live via odom_world_drift under fuse; static fallback) that contradicted the page's own prose.

Verified, not a defect (measured, not assumed)

  • "hold mis-fires at 50 Hz": the mecanum controller publishes odom at ~508 Hz (measured), ignoring publish_rate: 50.0, so the 15 ms hold-gap never fires mid-drive.
  • "source freeze if both controllers publish": only the active controller publishes odom (the inactive one is silent), so the two sources never interleave.

Deferred (works as-is; retuning needs re-validation)

  • Slip covariance ramp is conservative at wz_max=0.6 (tops ~0.13 vs kSpinCov=0.2); odom_world_drift composes est/base sampled at slightly different instants (self-corrects via AMCL).
  • Unit tests for the SE(2) / covariance pure functions (flagged by two reviewers) — recommended follow-up.

Pre-existing CI blocker (not introduced by this pass)

The hangar_sim integration test is red on plan_path_along_surface (×2): "Cancel for objective did not return within [timeout]". It fails on the CI baseline before these fixes, and the PR changes neither that objective nor the test — so it stems from the runtime config (fuse on by default). It correlates with the added localization CPU load slowing the sim, consistent with the CPU-bound caveat in the description, but is not definitively isolated. (solution_move_forward_2m passes on CI; its local failure was a CPU flake.) Needs a decision before merge.

@bkanator

Copy link
Copy Markdown
Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py`:
- Around line 190-193: Update the state estimator launch configuration
associated with the use_fuse argument to pass the use_sim_time launch value as
the node parameter {"use_sim_time": use_sim_time} alongside the existing
fuse.yaml configuration, ensuring Fuse uses the simulation clock.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 137c1df9-a38e-4fc2-bd73-ece87806e72f

📥 Commits

Reviewing files that changed from the base of the PR and between 153262c and b3952a6.

📒 Files selected for processing (10)
  • src/hangar_sim/CMakeLists.txt
  • src/hangar_sim/config/control/picknik_ur.ros2_control.yaml
  • src/hangar_sim/config/fuse/fuse.yaml
  • src/hangar_sim/description/picknik_ur_mujoco_ros2_control.xacro
  • src/hangar_sim/description/ur5e_ridgeback.xml
  • src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py
  • src/hangar_sim/package.xml
  • src/hangar_sim/params/nav2_params.yaml
  • src/hangar_sim/script/odom_world_drift.py
  • src/hangar_sim/src/slip_aware_odom.cpp

Comment thread src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

Widen the likelihood_field sensor model (sigma_hit 0.1->0.25, z_hit 0.9->0.65,
z_rand 0.1->0.3) so obstacles that are not in the map read as outliers instead of
spreading the particle cloud and lurching the pose, and score the full merged scan
(max_beams 180->720). Fixes catastrophic divergence near the unmapped picking boxes
(~18.8 m -> ~9 cm median) while holding the fuselage route (~8 cm / 0.77 deg).
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

…bustness

AMCL's scan-match goes degenerate where the base hugs the smooth fuselage
(slide-along ambiguity) or crosses the transient unmapped boxes: the pose
estimate teleports and the map flips, and with likelihood_field it never
recovers -- the robot is left unable to plan from a garbage pose.

Add amcl_odom_gate, which holds the last good map->odom and coasts on fuse's
odometry (locally trustworthy) through those zones, then blends back once AMCL
is trustworthy again. A large innovation -- gated on both position and yaw -- is
neither hard-accepted nor hard-rejected: it goes provisional and is accepted
only if it persists at one pose for persist_time (a valid correction persists;
an ambiguity teleport thrashes), so a real large correction such as recovering
from a bad initial-pose seed still gets through. Particle-spread hysteresis is a
second, independent hold trigger. AMCL runs with tf_broadcast:=false so the gate
is the sole map->odom publisher.

The pure decision logic (detail::updateGate) is split into a header with no
ROS/TF deps and unit-tested for every case: transparent tracking, thrashing
teleport held, yaw-only teleport held, persistent correction accepted (and held
until the timer elapses), spread hysteresis, and the SE(2) transform algebra.

Plane-nose stress (goal 0.3 m off the fuselage): 75% permanent-cascade failures
with likelihood_field -> ~10% transient/recovered with the gate. Boxes contained,
no strand.

Also: remove the now-superseded per-objective SetInitialPose reseed from the
clicked-point Objectives (slip_aware_odom's cross-controller re-anchoring covers
the whole-body<->nav handoff it was added for), and complete the truncated BSD
headers on slip_aware_odom.cpp and odom_world_drift.py that ament_copyright
rejected as license=unknown.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bkanator
bkanator force-pushed the feat/19667-fuse-odometry branch from 995a373 to e9fa7da Compare July 23, 2026 23:20

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py (1)

401-411: 🗄️ Data Integrity & Integration | 🟠 Major

fuse_state_estimator still doesn't pass use_sim_time — this was already flagged in a prior review round and appears unaddressed here.

fuse_state_estimator only loads fuse.yaml; with use_fuse now defaulting to true, Fuse stays on the wall clock while the rest of the sim stack (including the new odom_world_drift, slip_aware_odom, and amcl_odom_gate nodes just above, which now all correctly receive {"use_sim_time": use_sim_time}) runs on sim time. This is the same gap previously raised on state_estimator.

Suggested fix
     fuse_state_estimator = Node(
         package="fuse_optimizers",
         executable="fixed_lag_smoother_node",
         name="state_estimator",
         parameters=[
-            PathJoinSubstitution([hangar_sim_pkg, "config", "fuse", "fuse.yaml"])
+            PathJoinSubstitution([hangar_sim_pkg, "config", "fuse", "fuse.yaml"]),
+            {"use_sim_time": use_sim_time},
         ],
         output="screen",
         condition=IfCondition(LaunchConfiguration("use_fuse")),
     )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py` around
lines 401 - 411, Update the fuse_state_estimator Node parameters to include the
launch use_sim_time value alongside fuse.yaml, matching the {"use_sim_time":
use_sim_time} configuration used by the surrounding simulation nodes. Preserve
the existing package, executable, condition, and output settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py`:
- Around line 340-349: The launch configuration must provide a map->odom
publisher when localization is enabled and use_fuse is false. Update the
amcl_odom_gate/AMCL launch conditions or add a fallback publisher so the
use_fuse=false localization path receives map->odom, while preserving the
existing static_tf_map_to_odom behavior for localization=false.

In `@src/hangar_sim/package.xml`:
- Around line 44-48: Add the missing test dependency declaration for
ament_cmake_gmock in src/hangar_sim/package.xml, alongside the existing
dependency entries, so the test_amcl_odom_gate.cpp gmock target configured by
ament_add_gmock has its required package dependency.

---

Outside diff comments:
In `@src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py`:
- Around line 401-411: Update the fuse_state_estimator Node parameters to
include the launch use_sim_time value alongside fuse.yaml, matching the
{"use_sim_time": use_sim_time} configuration used by the surrounding simulation
nodes. Preserve the existing package, executable, condition, and output
settings.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b7ab5b22-211b-48ce-90f4-5c1623ea3a3c

📥 Commits

Reviewing files that changed from the base of the PR and between f0634e4 and 995a373.

📒 Files selected for processing (11)
  • src/hangar_sim/CMakeLists.txt
  • src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py
  • src/hangar_sim/objectives/navigate_to_clicked_point.xml
  • src/hangar_sim/objectives/navigate_to_clicked_point_with_replanning.xml
  • src/hangar_sim/package.xml
  • src/hangar_sim/params/nav2_params.yaml
  • src/hangar_sim/script/odom_world_drift.py
  • src/hangar_sim/src/amcl_odom_gate.cpp
  • src/hangar_sim/src/amcl_odom_gate_logic.hpp
  • src/hangar_sim/src/slip_aware_odom.cpp
  • src/hangar_sim/test/test_amcl_odom_gate.cpp
💤 Files with no reviewable changes (2)
  • src/hangar_sim/objectives/navigate_to_clicked_point.xml
  • src/hangar_sim/objectives/navigate_to_clicked_point_with_replanning.xml
🚧 Files skipped from review as they are similar to previous changes (3)
  • src/hangar_sim/params/nav2_params.yaml
  • src/hangar_sim/script/odom_world_drift.py
  • src/hangar_sim/src/slip_aware_odom.cpp

Comment on lines +340 to +349
# Confidence-gated map->odom: holds AMCL's correction and rides fuse odom when the
# particle cloud spreads on unmapped obstacles (AMCL runs with tf_broadcast:=false).
amcl_odom_gate = Node(
condition=IfCondition(LaunchConfiguration("use_fuse")),
package="hangar_sim",
executable="amcl_odom_gate",
name="amcl_odom_gate",
output="log",
parameters=[{"use_sim_time": use_sim_time}],
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd nav2_params.yaml src/hangar_sim
rg -n "tf_broadcast" src/hangar_sim/params/nav2_params.yaml
rg -n "tf_broadcast|use_fuse" src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py

Repository: PickNikRobotics/moveit_pro_example_ws

Length of output: 853


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== launch around use_fuse/localization =="
sed -n '120,170p' src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py
echo
sed -n '180,360p' src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py

echo
echo "== nav2 params around tf_broadcast =="
sed -n '35,60p' src/hangar_sim/params/nav2_params.yaml

Repository: PickNikRobotics/moveit_pro_example_ws

Length of output: 10281


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p = Path('src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py')
text = p.read_text()
for needle in ['use_fuse', 'localization', 'static_tf_map_to_odom', 'amcl_odom_gate', 'RewrittenYaml', 'param_substitutions']:
    print(f'== {needle} ==')
    for i, line in enumerate(text.splitlines(), 1):
        if needle in line:
            start = max(1, i-3)
            end = min(len(text.splitlines()), i+6)
            for j in range(start, end+1):
                print(f'{j}:{text.splitlines()[j-1]}')
            print()
PY

Repository: PickNikRobotics/moveit_pro_example_ws

Length of output: 12422


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
p = Path('src/hangar_sim/params/nav2_params.yaml')
for i, line in enumerate(p.read_text().splitlines(), 1):
    if 'tf_broadcast' in line or 'amcl:' in line or 'map->odom' in line:
        start = max(1, i-5)
        end = min(len(p.read_text().splitlines()), i+5)
        print(f'== around line {i} ==')
        for j in range(start, end+1):
            print(f'{j}:{p.read_text().splitlines()[j-1]}')
        print()
PY

Repository: PickNikRobotics/moveit_pro_example_ws

Length of output: 2118


🏁 Script executed:

#!/bin/bash
set -euo pipefail
grep -RIn --line-number --context=3 -E 'use_fuse|localization|static_tf_map_to_odom|amcl_odom_gate|RewrittenYaml|param_substitutions' src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py src/hangar_sim/params/nav2_params.yaml

Repository: PickNikRobotics/moveit_pro_example_ws

Length of output: 19881


use_fuse:=false still needs a map->odom source. nav2_params.yaml forces amcl.tf_broadcast: false, and amcl_odom_gate only launches when use_fuse is true. That leaves use_fuse=false + localization=true with no map->odom publisher; static_tf_map_to_odom only covers localization:=false. Consider making the AMCL broadcast/gate conditional on use_fuse, or adding a fallback publisher for the localization-on path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hangar_sim/launch/sim/robot_drivers_to_persist_sim.launch.py` around
lines 340 - 349, The launch configuration must provide a map->odom publisher
when localization is enabled and use_fuse is false. Update the
amcl_odom_gate/AMCL launch conditions or add a fallback publisher so the
use_fuse=false localization path receives map->odom, while preserving the
existing static_tf_map_to_odom behavior for localization=false.

Comment on lines +44 to +48
<!-- Runtime deps for the installed Python nodes (odom_world_drift, odom_qos_relay).
geometry_msgs and tf2_ros are covered by the <depend> tags above. -->
<exec_depend>rclpy</exec_depend>
<exec_depend>sensor_msgs</exec_depend>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n "gmock|gtest|ament_add_gmock|ament_add_gtest|find_package" src/hangar_sim/CMakeLists.txt
rg -n "gmock|gtest" src/hangar_sim/package.xml

Repository: PickNikRobotics/moveit_pro_example_ws

Length of output: 603


Add the missing gmock test dependency

src/hangar_sim/package.xml is missing <test_depend>ament_cmake_gmock</test_depend> for test_amcl_odom_gate.cpp; src/hangar_sim/CMakeLists.txt already uses find_package(ament_cmake_gmock REQUIRED) and ament_add_gmock(...).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/hangar_sim/package.xml` around lines 44 - 48, Add the missing test
dependency declaration for ament_cmake_gmock in src/hangar_sim/package.xml,
alongside the existing dependency entries, so the test_amcl_odom_gate.cpp gmock
target configured by ament_add_gmock has its required package dependency.

@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

…latency

Halving the fixed-lag smoother window from 0.5 to 0.3 lowers fuse-direct
moving-yaw error (fuselage route, GT-referenced) from a 0.70deg median to
0.465deg -- under the 0.5deg target -- and shrinks the worst-case error
spike from 20deg to 5.9deg. 0.25 reaches a marginally lower median (0.426)
but reintroduces a rare ~125deg transient (shorter window, less averaging);
0.3 keeps the median win with a clean transient tail.

Validated no regression: fuselage route 40/40, box cluster 12/12 (offset
bounded ~10cm), close-plane repeated-nav bounded by the gate (no cascade).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bkanator
bkanator force-pushed the feat/19667-fuse-odometry branch from 5096b5f to 9071054 Compare July 24, 2026 15:40
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

…re spread

The map->odom gate's persistence check accepts a large correction that stays
put for persist_time, assuming a real correction persists while an ambiguity
teleport thrashes. But a confident-WRONG lock (AMCL sliding along a smooth
surface like the plane fuselage, where many poses give near-identical scans)
also persists at one pose -- with a severely spread particle cloud. Persistence
alone cannot tell it from a real correction, so the gate adopted the wrong pose
after ~20s and the estimate cascaded to meters of error with no recovery.

Add spread_accept_max (default 3.0 = 2x spread_hold): a persisted correction is
accepted only while spread is at/below the cap. The check is folded INTO the
persistence requirement (a spread excursion re-anchors the clock), so persist_time
must elapse with the cloud continuously tight -- a single-frame spread dip during
a wrong lock cannot ratchet the held pose toward it. A legitimate correction
converges so its spread drops below the cap and is still accepted (bad-seed
recovery preserved); a divergence stays wide, so the gate keeps coasting on odom
until the robot leaves the ambiguous region and AMCL re-locks with a tight cloud.

Because the gate is the sole map->odom broadcaster, also add two watchdogs so a
degraded state is never silent: a coast warning when it holds beyond 5s, and a
stale-input warning in the publish timer when /particle_cloud stops updating
(input starvation the onCloud-side check cannot see).

Validated: the organic plane-nose divergence that previously plateaued at 6.7m
for 30s+ now stays bounded (gate error median 9-10cm, p99 ~20cm through the same
stress; longest sustained >100cm run is a single 0.1s glitch, was 20s+). Five new
unit tests cover the rejected wrong-lock, the preserved recovery, the threshold
boundary, the full-window bound, and the anti-ratchet oscillating-spread case;
all 14 gate tests pass.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bkanator
bkanator force-pushed the feat/19667-fuse-odometry branch from ddaba6c to 0a17521 Compare July 25, 2026 17:56
@github-actions

Copy link
Copy Markdown

MoveIt Pro Example WS - Objectives Integration Test Report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant