Skip to content

Configure PiBT planning grid resolution - #51

Merged
arjo129 merged 5 commits into
open-rmf:mainfrom
SamuelFoo:feat/configurable-planning-parameters
Sep 2, 2026
Merged

Configure PiBT planning grid resolution#51
arjo129 merged 5 commits into
open-rmf:mainfrom
SamuelFoo:feat/configurable-planning-parameters

Conversation

@SamuelFoo

@SamuelFoo SamuelFoo commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

New feature implementation

Implemented feature

This PR makes the PiBT planning grid resolution configurable through a read-only ROS parameter.

The existing 1.0 m resolution remains the default, so current deployments retain their behavior unless the parameter is overridden.

Implementation description

This PR:

  • adds the planning_grid_resolution parameter to rmf_path_server
  • applies the configured resolution to mapped and mapless planning
  • rejects non-finite and non-positive resolutions
  • preserves coarser source-map resolutions instead of upsampling them
  • adds unit tests for configured and invalid resolutions
  • documents the parameter in the path server README

Partially addresses #47.

test_map_subscription.rs was edited to pass rustfmt tests.

GenAI Use

We follow OSRA's policy on GenAI tools

  • I used a GenAI tool in this PR.
  • I did not use GenAI

Generated-by: GPT-5

Declare startup parameters for the PIBT grid resolution and global robot footprint. Apply the configured resolution to mapped and mapless planning while preserving the existing defaults.

Signed-off-by: SamuelFoo <fooenzesamuel@gmail.com>
@mxgrey mxgrey added this to PMC Board Aug 27, 2026
@github-project-automation github-project-automation Bot moved this to Inbox in PMC Board Aug 27, 2026
@SamuelFoo SamuelFoo changed the title Feat/configurable planning parameters Configure planning and route blockage parameters Aug 27, 2026
@SamuelFoo
SamuelFoo marked this pull request as ready for review August 27, 2026 09:01

@arjo129 arjo129 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we consider splitting this up into smaller PRs? Lets merge your PIBT stuff in. The executor stuff may need a bit more iteration.

use std::sync::Arc;

const MIN_PLANNING_RESOLUTION: f32 = 1.0;
pub const DEFAULT_PLANNING_GRID_RESOLUTION: f64 = 1.0;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
pub const DEFAULT_PLANNING_GRID_RESOLUTION: f64 = 1.0;
pub(crate) const DEFAULT_PLANNING_GRID_RESOLUTION: f64 = 1.0;

Keep it public within the crate. Don't make it pub to the entire world.

Comment thread path_server/rmf_path_server/src/main.rs Outdated
.description("Minimum PIBT grid-cell size in metres")
.read_only()?;
let robot_footprint_radius = node
.declare_parameter("robot_footprint_radius")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Lets not introduce this parameter yet. There is a discussion for this here: #3

blockage_debounce: Duration::from_secs_f64(DEFAULT_BLOCKAGE_DEBOUNCE_SECONDS),
replan_cooldown: Duration::from_secs_f64(DEFAULT_REPLAN_COOLDOWN_SECONDS),
occupied_threshold: DEFAULT_OCCUPIED_THRESHOLD as i8,
allocation_grid_resolution: DEFAULT_ALLOCATION_GRID_RESOLUTION as f32,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ive got a slightly different approach for this in a throw away branch where allocation_grid_resolution is directly derived from the /map topic:

pub fn handle_map(&mut self, msg: OccupancyGrid) {

For the spatio-temporal allocation it makes sense to go with a much higher resolution.

Remove the global robot footprint parameter while its interface is still under discussion. Preserve the existing 0.49 m fallback behavior.

Signed-off-by: SamuelFoo <fooenzesamuel@gmail.com>
Keep the default grid resolution private to the library crate. Read the executable's parameter default from PibtPlanner instead of exporting the constant.

Signed-off-by: SamuelFoo <fooenzesamuel@gmail.com>
Document the planning_grid_resolution startup parameter and align its help text with the repository's terminology.

Signed-off-by: SamuelFoo <fooenzesamuel@gmail.com>
Signed-off-by: SamuelFoo <fooenzesamuel@gmail.com>
@SamuelFoo
SamuelFoo force-pushed the feat/configurable-planning-parameters branch from a144539 to b74847d Compare September 1, 2026 01:56
@SamuelFoo SamuelFoo changed the title Configure planning and route blockage parameters Configure PiBT planning grid resolution Sep 1, 2026
@SamuelFoo

Copy link
Copy Markdown
Contributor Author

Made the changes and updated the PR description. Ready for review!

@arjo129 arjo129 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Before I merge it do you mind adding a test/demo?

@arjo129
arjo129 merged commit d36a706 into open-rmf:main Sep 2, 2026
3 checks passed
@github-project-automation github-project-automation Bot moved this from Inbox to Done in PMC Board Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants