Skip to content

Add omni (SE2) analytic expansion to SmacPlannerLattice#5965

Open
ManMan88 wants to merge 4 commits intoros-navigation:mainfrom
ManMan88:fix/omni-analytic-expansion
Open

Add omni (SE2) analytic expansion to SmacPlannerLattice#5965
ManMan88 wants to merge 4 commits intoros-navigation:mainfrom
ManMan88:fix/omni-analytic-expansion

Conversation

@ManMan88
Copy link

@ManMan88 ManMan88 commented Feb 17, 2026

Basic Info

Info
Ticket(s) this addresses #2683, related: #5231, #4262
Primary OS tested on Ubuntu 24.04
Robotic platform tested on Simulated rectangled shape holonomic AMR
Does this PR contain AI generated software? Yes, not marked inline
Was this PR description generated by AI software? No

Description of contribution in a few bullet points

  • Added detection of motion_model: "omni" from the lattice metadata to select SE2StateSpace instead, which produces straight line paths with linear heading interpolation
  • Added MotionModel::OMNI enum value with string conversion support
  • Applied the same omni detection in the distance heuristic precomputation (reordered metadata loading to occur before state space selection)
  • Skip turning-radius refinement in refineAnalyticPath() for omni since SE2 paths don't depend on turning radius

Description of documentation updates required from your changes

  • New OMNI motion model enum value, maybe add a note in the SmacPlannerLattice configuration docs
  • No new parameters added, the omni motion model is auto-detected from the existing motion_model field in the lattice primitives file

Description of how this change was tested

  • Added 2 unit tests: test_omni_selects_se2_state_space and test_non_omni_selects_reeds_shepp
  • Manually tested with a holonomic AMR configuration and confirmed straight-line analytic expansions instead of arcs.

Future work that may be required in bullet points

  • Backport to jazzy (code structure differs slightly — precomputeDistanceHeuristic lives in node_lattice.cpp and analytic refinement is inline)

@SteveMacenski
Copy link
Member

@ManMan88 can you please properly fill in the PR template?

@SteveMacenski
Copy link
Member

Anything else we should update to optimize for omni platforms?

I think maybe this needs to be updated at least:

if (motion_model != MotionModel::STATE_LATTICE) {
& https://github.com/ManMan88/navigation2/blob/09d46dec9ae0ce662f7ea7fdbfcd78004e097b4d/nav2_smac_planner/src/analytic_expansion.cpp#L61-L62

Seems to me like this may not be functional (?)

@codecov
Copy link

codecov bot commented Feb 17, 2026

Codecov Report

❌ Patch coverage is 47.61905% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
nav2_smac_planner/src/smac_planner_lattice.cpp 30.00% 7 Missing ⚠️
nav2_smac_planner/src/node_lattice.cpp 0.00% 4 Missing ⚠️
Files with missing lines Coverage Δ
...ac_planner/include/nav2_smac_planner/constants.hpp 100.00% <ø> (ø)
nav2_smac_planner/src/analytic_expansion.cpp 89.01% <100.00%> (ø)
nav2_smac_planner/src/distance_heuristic.cpp 87.09% <100.00%> (+0.43%) ⬆️
nav2_smac_planner/src/node_lattice.cpp 91.55% <0.00%> (-1.24%) ⬇️
nav2_smac_planner/src/smac_planner_lattice.cpp 88.39% <30.00%> (-1.56%) ⬇️

... and 13 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ManMan88
Copy link
Author

Sorry about the missing faulty PR format, I'll change it to the correct format based on the template.
Regarding your comment, you are correct, I added OMNI to the if statements of the motion model guards.

Copy link
Collaborator

@mini-1235 mini-1235 left a comment

Choose a reason for hiding this comment

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

Could you also attach some screenshots / videos to help us better understand how the results differ after your changes?

SearchInfo & search_info)
{
if (motion_model != MotionModel::STATE_LATTICE) {
if (motion_model != MotionModel::STATE_LATTICE && motion_model != MotionModel::OMNI) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is currently hardcoded to STATE_LATTICE

_motion_model = MotionModel::STATE_LATTICE;

Copy link
Author

@ManMan88 ManMan88 Feb 18, 2026

Choose a reason for hiding this comment

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

Yes sure, I'll add some before / after images showing the planned path for some scenarios.
The purple arrow is the requested goal.

(Just a clarification, these manual tests are for similar fixes made on the jazzy branch, should I make a PR for it as well? see [this commit])(ManMan88@f6001cc)

before:
Screenshot from 2026-02-18 18-06-50

after:
Screenshot from 2026-02-18 18-00-33

before:
Screenshot from 2026-02-18 18-05-11

after:
Screenshot from 2026-02-18 18-01-20

before:
Screenshot from 2026-02-18 18-04-05

after:
Screenshot from 2026-02-18 18-01-44

Copy link
Author

@ManMan88 ManMan88 Feb 18, 2026

Choose a reason for hiding this comment

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

The release version make weird paths as I used parameters that tries to force omni paths, but it didn't work.
I used the following parameters for the fix version (2 parameters were changed from the ones used with the release version):

planner_server:
  ros__parameters:
    expected_planner_frequency: 5.0
    planner_plugins: ["GridBased"]
    
    GridBased:
      # Use SmacPlannerLattice plugin
      plugin: "nav2_smac_planner::SmacPlannerLattice"
      
      # --- Core Planning Controls ---
      allow_unknown: true                       # Allow planning through unknown space
      tolerance: 0.25                           # Goal tolerance for planning (meters)
      max_iterations: 1000000                   # Maximum planning iterations
      max_on_approach_iterations: 1000          # Max iterations when approaching goal
      max_planning_time: 2.0                    # Maximum time for planning (seconds)
      
      # --- Heuristic Penalties (tune as-needed) ---
      # These control the cost of different motion types
      analytic_expansion_ratio: 3.5              # (**USED 200** in the release version)  Default — enables analytic expansion with omni SE2 paths
      analytic_expansion_max_length: 10.0       # (**USED 0.1** in the release version) Moderate length to limit abrupt heading transitions
      analytic_expansion_max_cost: 200.0        # Max cost for analytic expansion
      analytic_expansion_max_cost_override: false  # Override max cost check

      reverse_penalty: 1.0                      # No penalty — holonomic can reverse freely
      change_penalty: 0.05                      # Penalty for changing direction
      non_straight_penalty: 1.0                 # No penalty — lateral strafing is native for holonomic
      cost_penalty: 2.0                         # Penalty multiplier for costmap cost
      rotation_penalty: 1.0                     # No penalty — holonomic can pivot freely
      retrospective_penalty: 0.015              # Penalty for retrospective search
      
      # --- Lattice-Specific Parameters ---
      # Path to the motion primitives file (MUST be absolute path at runtime)
      # This will be set via launch file to ensure proper path resolution
      lattice_filepath: ""
      
      lookup_table_size: 20.0                   # Size of lookup table for heuristic
      cache_obstacle_heuristic: true            # Cache obstacle heuristic (big speedup if map is mostly static)
      allow_reverse_expansion: true             # Allow reverse motion primitives
      
      # --- Path Smoothing ---
      smooth_path: true                         # Enable path smoothing
      smoother:
        max_iterations: 800                     # Max smoothing iterations
        w_smooth: 0.3                           # Weight for smoothness
        w_data: 0.2                             # Weight for data fidelity
        tolerance: 1.0e-10                      # Convergence tolerance
        do_refinement: true                     # Enable refinement
        refinement_num: 2                       # Number of refinement passes
      
      # --- Goal Heading Search ---
      # Controls how the planner handles goal heading
      # "DEFAULT" = use specified goal heading
      # "ALL_DIRECTION" = search for best goal heading from all directions
      goal_heading_mode: "DEFAULT"
      coarse_search_resolution: 1               # Resolution for coarse search (if using ALL_DIRECTION)

@SteveMacenski
Copy link
Member

SteveMacenski commented Feb 18, 2026

I'm going to let @mini-1235 do the first few reviews to catch the big stuff but I wanted to thank you for taking this one. This is a much more elegant method than I had in mind and works much better.

I'd love for you to use some of those before/after in the docs.nav2.org migration guide to highlight this. This is a great contribution 💯

Copy link
Collaborator

@mini-1235 mini-1235 left a comment

Choose a reason for hiding this comment

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

I left a few comments on items that need to be reverted. Basically, the _motion_model is hardcoded to STATE_LATTICE, so it can never be OMNI. But otherwise the changes looks good to me!

_motion_model = MotionModel::STATE_LATTICE;

Once this PR is merged, feel free to open a backport PR to jazzy

@SteveMacenski SteveMacenski linked an issue Mar 12, 2026 that may be closed by this pull request
4 tasks
@SteveMacenski
Copy link
Member

@ManMan88 any updates? I'd love to get this in!

@ManMan88
Copy link
Author

Sorry, I've been quite busy.
I'll take a look and make the required fixes this week.

@ManMan88
Copy link
Author

ManMan88 commented Mar 18, 2026

@mini-1235 I agree with your comments.
I updated thePR with a commit that reverted these changes.

Should I also add a similar fix for the jazzy branch?

@SteveMacenski
Copy link
Member

SteveMacenski commented Mar 19, 2026

Should I also add a similar fix for the jazzy branch?

Sure!

Also, can you add to the migration guide on docs.nav2.org this addition for omni support? Pick one of your before and after pictures for a side-by-side illustration I think would be useful.

Please sign off the commit with DCO. Check that failed job for how-to.

Finally - any other additions for optimal OMNI support or does this button that up?

@ManMan88
Copy link
Author

ManMan88 commented Mar 20, 2026

Finally - any other additions for optimal OMNI support or does this button that up?

I consulted opus 4.6, it found these issues:

  1. Smoother uses non-holonomic mode for OMNI — SmootherParams::holonomic_ defaults to false and is never set to true for OMNI. This means Dubins-based boundary conditions are enforced on an omni robot's path during smoothing. Compare with smac_planner_2d.cpp:118 which explicitly sets params.holonomic_ = true. This is the most impactful gap.
  2. allow_reverse_expansion can break OMNI — If a user sets allow_reverse_expansion=true with an OMNI lattice, the planner will append reversed-heading primitives with a reverse_penalty, which is meaningless for holonomic robots. Currently safe only because it defaults to false, but there's no guard preventing misconfiguration.

So I'll apply a fix for these as well.

@mergify
Copy link
Contributor

mergify bot commented Mar 20, 2026

This pull request is in conflict. Could you fix it @ManMan88?

@ManMan88 ManMan88 force-pushed the fix/omni-analytic-expansion branch from 767594b to 94c721b Compare March 20, 2026 11:04
When lattice file specifies motion_model: omni, use SE2StateSpace
for analytic expansion instead of Dubins/Reeds-Shepp curves.
This produces straight-line paths for holonomic robots instead of
arc-based paths with unnecessary rotations.

Fixes ros-navigation#2683
Related: ros-navigation#5231, ros-navigation#4262

Signed-off-by: Ron Danon <rondanon@gmail.com>
Set smoother to holonomic mode when lattice metadata indicates omni,
avoiding incorrect Dubins boundary conditions on holonomic paths.
Disable allow_reverse_expansion for omni robots with a warning, as
the concept of reverse is meaningless for holonomic motion.

Fixes ros-navigation#2683
Related: ros-navigation#5231, ros-navigation#4262

Signed-off-by: Ron Danon <rondanon@gmail.com>
@ManMan88 ManMan88 force-pushed the fix/omni-analytic-expansion branch from 94c721b to 46cb204 Compare March 20, 2026 11:08
Copy link
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

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

LGTM! Just the docs update I asked for which I think is good to highlight your work, and we can merge this!

Good catch on the smoother, I didn't think about that since this PR didn't touch it

@ManMan88
Copy link
Author

ManMan88 commented Mar 21, 2026

I updated the docs in this PR: ros-navigation/docs.nav2.org#891
Please let me know if this is sufficient.

@ManMan88 ManMan88 requested a review from mini-1235 March 21, 2026 11:19
@SteveMacenski
Copy link
Member

I'll take a look now. Can you also add in a bit of the missing test coverage for the omni case in configuration / parameters to cover the lines added? https://app.codecov.io/gh/ros-navigation/navigation2/pull/5965?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=checks&utm_campaign=pr+comments&utm_term=ros-navigation

These seem relatively easy to add

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.

[Smac State Lattice] Support Omni Lateral Motions

3 participants