Skip to content

docs(costmap-filters): add Zone Parameter Filter configuration page - #907

Merged
SteveMacenski merged 16 commits into
ros-navigation:masterfrom
aki1770-del:feat/zone-parameter-filter-docs
Aug 10, 2026
Merged

docs(costmap-filters): add Zone Parameter Filter configuration page#907
SteveMacenski merged 16 commits into
ros-navigation:masterfrom
aki1770-del:feat/zone-parameter-filter-docs

Conversation

@aki1770-del

@aki1770-del aki1770-del commented Apr 25, 2026

Copy link
Copy Markdown
Contributor

Basic Info

Info Please fill out this column
Ticket(s) this addresses Companion to ros-navigation/navigation2#6104 (closes ros-navigation/navigation2#6080)
Does this PR contain AI-generated software? Yes — AI-assisted, with the human author (aki1770-del) reviewing every change before commit.

Description of contribution in a few bullet points

  • New configuration page configuration/packages/costmap-plugins/zone_parameter_filter.rst documenting all 10 parameters of the new ZoneParameterFilter costmap filter plugin (introduced by navigation2#6104). Includes a fully-described YAML example.
  • New entry added to the Costmap Filters Parameters toctree in configuration/packages/configuring-costmaps.rst so the new page renders in the navigation.
  • New row added to the Costmap Filters table in plugins/index.rst with the corresponding link to the plugin source. Table column 1 widened from 20 to 26 characters to fit the longer plugin name.

Notes for review

  • This PR depends on navigation2#6104 being accepted in some form. If the upstream PR is reshaped (e.g., parameter renames, schema changes), this docs PR will be updated to track. Happy to hold this PR until the upstream code PR is closer to merge if that's the maintainer preference.
  • The YAML example mirrors the example in navigation2#6104's PR body and the Example Fully-Described YAML block in the new doc page.

@aki1770-del
docs(costmap-filters): add Zone Parameter Filter configuration page
ef89e55 
Companion documentation for ros-navigation/navigation2#6104, which
introduces the new ZoneParameterFilter costmap filter plugin.

- Add configuration/packages/costmap-plugins/zone_parameter_filter.rst
  documenting all parameters: enabled, filter_info_topic,
  transform_tolerance, target_nodes, state_ids, state_<N>.<node>.<param>,
  nominal_defaults.<node>.<param>, state_event_topic, on_unknown_state,
  on_param_set_failure. Includes a fully-described YAML example.
- Add the new file to the Costmap Filters Parameters toctree in
  configuration/packages/configuring-costmaps.rst.
- Add Zone Parameter Filter row to the Costmap Filters table in
  plugins/index.rst (table widened to fit the longer plugin name).

Co-Authored-By: Claude and aki1770-del <aki1770@gmail.com>
Signed-off-by: Akihiko Komada <aki1770@gmail.com>
@aki1770-del aki1770-del mentioned this pull request Apr 25, 2026
Merged
8 tasks
@aki1770-del
docs(zone_parameter_filter): add Common Pitfalls section
9e709b9 
Five operational sharp edges that surfaced during nav2#6104 design +
post-PR audit:

- Target-node typos go silent at config-load until first transition
- State-to-state transitions only apply the new state's overrides
  (only state 0 resets to nominal_defaults)
- Hot path is non-blocking by design (no wait_for_service)
- Effective state-ID range is [1, 127] (int8_t mask transport)
- Longest-prefix matching with deterministic length-descending sort

Co-Authored-By: Claude and aki1770-del <aki1770@gmail.com>
Signed-off-by: Akihiko Komada <aki1770@gmail.com>
SteveMacenski
SteveMacenski reviewed Apr 27, 2026
View reviewed changes
Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
@aki1770-del
docs(zone_parameter_filter): rework intro + trim Common Pitfalls + re…
7b002d7 
…move on_unknown_state

Addresses Steve Macenski review on PR ros-navigation#907:
- L11 (mechanical phrasing): rewrote the intro paragraph to 4 lines,
  stripped marketing-style framing.
- L18 (int8_t/OCC_GRID_UNKNOWN parenthetical): removed; range stated as
  [1, 127] plainly.
- L24 (AsyncParametersClient configuration paragraph): removed — not a
  configuration detail.
- L70 (target_nodes longest-prefix-match prose): stripped from
  Description; the matching logic stays in code (see PR #6104 reply).
- L116 (on_unknown_state parameter docs entry + YAML example line):
  removed for coherence with PR #6104 commit e61099a, which removed the
  parameter from the code (always-throw on unknown mask state per the
  C.2.a part of the review). Without this, the docs would describe a
  parameter that no longer exists.
- L190 (Common Pitfalls section): renamed to "Notes" and trimmed from
  5 bullets to 1 (the state-to-state transition behavior, which is a
  real configuration consideration).

Items deferred to paired commits as the remaining PR #6104 changes ship:
- L94 (nominal_defaults framing — depends on auto-capture refactor)
- L114 (state_event_topic default — depends on code-side default flip)
- L136 (on_param_set_failure — pushback substance restated in reply;
  docs default-flip pairs with code commit when it lands)

Signed-off-by: Akihiko Komada <aki1770@gmail.com>
@aki1770-del

aki1770-del commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for reviewing the documents.

I pushed changes in commit 7b002d7 that addressed the items not dependent on the in-flight code changes:

  1. L11 - Introduction reworded to 4 lines, marketing prose removed.
  2. L18 - Parenthetical removed; range stated as [1, 127].
  3. L24 - AsyncParametersClient paragraph removed.
  4. L70 - Longest-prefix-match prose stripped from the target_nodes Description (matching logic itself stays, context in #6104 reply).
  5. on_unknown_state parameter entry and YAML example line removed for coherence with the C.2.a fix in #6104 (e61099a), which removes the parameter entirely.
  6. L190 - Common Pitfalls renamed to "Notes" and trimmed from 5 bullets to 1 (the state-to-state transition behavior, which is a real configuration consideration). Happy to drop the section entirely if even the one bullet feels off.

Three items were deferred to paired commits as the remaining #6104 changes land:

  1. L94 (nominal_defaults) - Flipping to genuinely-optional auto-captured-at-startup. Pairs with the init-time-client refactor on #6104.
  2. L114 (state_event_topic) - Defaulting to "zone_filter_state" and dropping the "Optional" framing. Pairs with the code-side default flip on #6104.
  3. L136 (on_param_set_failure) - Keeping the parameter, default-flipping to throw. The other filters in costmap_filters/ handle transient RPC/tf2 faults uniformly with throttled-warn-and-continue; collapsing all such transient classes to a stack-killing exception introduces a second-order failure mode where a single network blip takes the nav stack down. Throw-by-default matches your safety call; the configurable escape preserves the existing convention for transient classes. Happy to bundle into the unconditional removal if you'd rather have it consistent.

Please let me know if you have any questions or concerns regarding the changes made.

@aki1770-del
docs(zone_parameter_filter): paired updates for state_event_topic + o…
b0ef2bc 
…n_param_set_failure defaults

Pairs with PR #6104 commits db32b3c (state_event_topic default) and
1cf64e4 (on_param_set_failure default-flip).

- L114 (state_event_topic): default flipped from "" to "zone_filter_state";
  "Optional" framing dropped; description now states the publisher is
  always created.
- L136 (on_param_set_failure): default flipped from "warn" to "throw";
  description rewritten to explain the safety rationale (fail-safe on
  persistent faults) and the warn-escape for transient-RPC environments.
- Example YAML: state_event_topic value updated to match new default;
  on_param_set_failure value updated to "throw" with comment noting it
  is the new default.

Signed-off-by: Akihiko Komada <aki1770@gmail.com>
@aki1770-del
docs(zone_parameter_filter): trim nominal_defaults description for co…
aca0d41 
…nsistency

Pairs with the PR #6104 Theme B comment scrub and clarifies the answer
to Steve Macenski's L94 inline comment ("Can we not 'get' each value at
startup so we have those rather than configuring them?").

The honest answer is: we tried, and the underlying services::Client
FIFO race makes auto-capture unreliable. The original docs paragraph
already explained this; the production code keeps the declarative-YAML
approach. Trimmed the docs paragraph to match the more concise comment
shape on the cpp side after the PR #6104 Theme B scrub:

- Drops the "Optional but strongly recommended" hedge framing.
- Drops "One entry per parameter..." (self-evident from the parameter
  shape declaration above the Description).
- Keeps the race-condition reason in one sentence (the answer to
  Steve's question).
- Keeps the warn-on-gap behavior note.

Signed-off-by: Akihiko Komada <aki1770@gmail.com>
SteveMacenski
SteveMacenski reviewed May 8, 2026
View reviewed changes
Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
@SteveMacenski

SteveMacenski commented May 8, 2026

Copy link
Copy Markdown
Member

Other than the 2 open items; LGTM

aki1770-del added 2 commits May 9, 2026 17:14
@aki1770-del
docs(zone_parameter_filter): document N-only reset on N→M transitions
5778b95 
Notes block previously admitted the cross-state leak; rewrite to describe
the new transition-time reset behavior shipped on nav2 PR #6104. Closes
ros-navigation#907 review thread 3211251415.

Signed-off-by: Akihiko Komada <aki1770@gmail.com>
@aki1770-del
docs(zone_parameter_filter): strengthen nominal_defaults declaration …
b52d2ee 
…rationale

Cite three defenses (FIFO-race, lifecycle service-readiness, async-drain
timing) rather than FIFO-only, and link to the Notes-block transition
semantics. Closes ros-navigation#907 review threads
3150280806 + 3211253512.

Signed-off-by: Akihiko Komada <aki1770@gmail.com>
@aki1770-del

aki1770-del commented May 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the round-2 review. The Notes-block has been rewritten in 5778b95c to describe the cross-state reset behavior shipped via ros-navigation/navigation2#6104 864d49b; the nominal_defaults description has been strengthened in b52d2ee8 to cite all three reasons (FIFO race, lifecycle service-readiness, async-drain timing) rather than FIFO alone, with the full engineering rationale on the nav2 PR reply.

SteveMacenski
SteveMacenski requested changes May 11, 2026
View reviewed changes
Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
@aki1770-del
docs(zone_parameter_filter): trim nominal_defaults rationale per review
6c04135 
Per @SteveMacenski feedback on PR ros-navigation#907 (TMI): drop the (1)(2)(3)
declared-vs-auto-captured rationale from the Description block; the
warning + N-only reset behavior remains documented in the Notes section
below.

Signed-off-by: Akihiko Komada <aki1770@gmail.com>
@mergify

mergify Bot commented Jul 9, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@aki1770-del
docs(zone_parameter_filter): refresh to current schema; add tutorial,…
98340f5 
… migration and plugins entries

Updates the configuration page to the reworked #6104 schema (states ->
<state>.{id, setpoints} -> {node, parameter, value}; nominal_defaults;
state_event_topic) and adds the remaining invited artifacts: the
Navigating with Zone Parameter Overrides tutorial, the Kilted migration
entry, the Navigation Plugins table row, and the filter-info type 4 line.

Signed-off-by: Akihiko Komada <aki1770@gmail.com>
@aki1770-del

aki1770-del commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Refreshed for the reworked schema and extended to the full docs set discussed in ros-navigation/navigation2#6104:

  • configuration page rewritten to the current parameters (states -> <state>.{id, setpoints} -> {node, parameter, value}, nominal_defaults, state_event_topic), matching the nav2 PR head 80ad542
  • new tutorial: Navigating with Zone Parameter Overrides (mask values -> state ids -> setpoints -> nominal defaults, with a verification step watching zone_filter_state and ros2 param get)
  • Kilted migration entry, Navigation Plugins table row, and the - 4: zone parameter filter line on the filter-info server page
  • the two open review threads are superseded by the rewrite: parameter descriptions are one sentence each, with a single Notes block

No images yet: mask/RViz screenshots will follow from a demo run rather than being mocked up.

SteveMacenski
SteveMacenski reviewed Jul 27, 2026
View reviewed changes

@SteveMacenski SteveMacenski 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.

Generally looks good, but seems like there's a merge conflict and some issues in resolving that

Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
Comment thread configuration/packages/configuring-costmaps.rst

@SteveMacenski SteveMacenski Jul 27, 2026

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.

This appears to contain other PR's commits

Comment thread migration/Kilted.rst

@SteveMacenski SteveMacenski Jul 27, 2026

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.

This appears to contain other PR's commits

Comment thread migration/Kilted.rst Outdated
Comment thread plugins/index.rst

@SteveMacenski SteveMacenski Jul 27, 2026

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.

This appears to contain other PR's commits

Comment thread plugins/index.rst

@SteveMacenski SteveMacenski Jul 27, 2026

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.

This appears to contain other PR's commits

Comment thread tutorials/index.rst

@SteveMacenski SteveMacenski Jul 27, 2026

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.

This appears to contain other PR's commits

Comment thread configuration/packages/costmap-plugins/zone_parameter_filter.rst Outdated
Comment thread tutorials/docs/navigation2_with_zone_parameter_filter.rst Outdated
Comment thread tutorials/docs/navigation2_with_zone_parameter_filter.rst Outdated
SteveMacenski and others added 6 commits July 27, 2026 10:56
@SteveMacenski
Update configuration/packages/costmap-plugins/zone_parameter_filter.rst
da4b200 
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
@SteveMacenski
Update migration/Kilted.rst
7fec657 
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
@SteveMacenski
Update tutorials/docs/navigation2_with_zone_parameter_filter.rst
910993b 
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
@SteveMacenski
Update tutorials/docs/navigation2_with_zone_parameter_filter.rst
dfa8e95 
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
@SteveMacenski
Update configuration/packages/costmap-plugins/zone_parameter_filter.rst
d865dd7 
Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
@aki1770-del
Merge remote-tracking branch 'upstream/master' into zpf-docs-merge
d97dd82 
Signed-off-by: Akihiko Komada <aki1770@gmail.com>

# Conflicts:
#	migration/Kilted.rst
@mergify

mergify Bot commented Aug 8, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@aki1770-del
docs: restore trailing newline in zone_parameter_filter.rst
79c2448 
pre-commit's end-of-file-fixer reports a missing newline at the end of
configuration/packages/costmap-plugins/zone_parameter_filter.rst.

The file's contents are unchanged by the merge with master — it is
byte-identical before and after. The merge brought in the current
pre-commit workflow, which is what surfaced the missing newline.

This adds the one missing newline so the check passes.

Signed-off-by: Akihiko Komada <aki1770@gmail.com>
@mergify

mergify Bot commented Aug 10, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@SteveMacenski
SteveMacenski merged commit 187cad2 into ros-navigation:master Aug 10, 2026
6 checks passed
@mergify mergify Bot mentioned this pull request Aug 10, 2026
Merged
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveMacenski SteveMacenski SteveMacenski approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Feature: Add SurfaceConditionFilter costmap filter plugin

2 participants

@aki1770-del @SteveMacenski