Skip to content

Give box/footprint/polygon filters unique private node names - #266

Open
rafal-gorecki wants to merge 2 commits into
ros-perception:rollingfrom
rafal-gorecki:fix/internal-node-name-collision-214
Open

Give box/footprint/polygon filters unique private node names#266
rafal-gorecki wants to merge 2 commits into
ros-perception:rollingfrom
rafal-gorecki:fix/internal-node-name-collision-214

Conversation

@rafal-gorecki

Copy link
Copy Markdown

Fixes #214.

box_filter, footprint_filter and polygon_filter each spin up their own
internal node (for TF, and for polygon_filter also pub/sub/param callbacks).
That internal node used a fixed name, so a process-wide bare
-r __node:=<name> remap (e.g. Node(name="laser_filter") in a launch file)
silently renames it too, colliding with the parent filter-chain node and
producing Publisher already registered for node name: 'laser_filter',
exactly as reported here and in #214 (comment).

Each internal node now gets a name unique to the filter instance, pinned via
its own local -r __node:=<name> argument — that survives a process-wide
bare remap since node-specific rules take precedence over the wildcard one.

This follows the same approach @jonbinney used in #248 for box_filter/
polygon_filter (opened against #231), and extends it to footprint_filter,
which #248 doesn't cover but is affected by the same root cause.

Tested against the exact repro from the issue (scan_to_scan_filter_chain,
--remap __node:=laser_filter, box_filter via params file): the
"Publisher already registered" warning and the node-name collision are
gone, and the box filter's internal node keeps a unique name in
ros2 node list.

These filters spin up their own internal node for TF and (for polygon)
pub/sub/param-callback needs. That internal node used a fixed name, so
a process-wide "-r __node:=<name>" remap (e.g. Node(name="laser_filter")
in a launch file) silently renamed it too, colliding with the parent
filter-chain node under the same name and breaking rosout logging.

Give each internal node a name unique to the filter instance, pinned
via its own local "-r __node:=<name>" argument so it survives a
process-wide bare remap.

Fixes ros-perception#214
@mergify

mergify Bot commented Aug 14, 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

box_filter, footprint_filter and polygon_filter each construct their
TransformListener with the buffer-only convenience constructor, which
spins up yet another hidden node (and dedicated thread) purely to
subscribe to /tf and /tf_static, on top of the private node these
filters already carry. Pass that private node through instead, so
there's one node per filter, not two, matching the same cleanup
already done for the top-level filter-chain node in
1801589 ("Re-use the main filter node for the TransformListener").
@rafal-gorecki

Copy link
Copy Markdown
Author

A note on why this also drops rclcpp_lifecycle::LifecycleNode in favor of a plain rclcpp::Node for the private per-filter node (box/footprint/polygon):

These classes never actually engage the lifecycle state machine — no on_configure/on_activate overrides, and nothing anywhere in the codebase calls trigger_transition() on them. configure() here is just filters::FilterBase's own interface method, unrelated to the lifecycle callback of the same name.

Despite that, the resulting node still advertises itself as a fully manageable lifecycle node (~/change_state, ~/get_state, ~/transition_event — confirmed via ros2 lifecycle nodes). Since nothing ever spins its executor, any real interaction with those services just hangs forever (confirmed: ros2 lifecycle get <node> never returns). So today it's a node that looks lifecycle-managed to any tooling that discovers it that way, but isn't — pure surface area for zero benefit. A plain rclcpp::Node gives these filters exactly what they use (logger, clock, pub/sub, param callback) without that trap, and matches the approach @jonbinney already took in #248 for box_filter/polygon_filter.

@rafal-gorecki

Copy link
Copy Markdown
Author

@jonbinney please review

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.

Creating multiple nodes when changing node name

1 participant