CostmapTopicCollisionChecker tf stamp fix

[robotechvision]

---

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	#2658
Primary OS tested on	(Ubuntu)
Robotic platform tested on	(RTV Caster, gazebo simulation of RTV Caster)

---

Description of contribution in a few bullet points

• Fixed footprint stamp rewriting by Costmap2DROS
• Added header retrieval to FootprintSubscriber
• Moved footprint "unorienting" from CostmapTopicCollisionChecker to Footprint subscriber and made it use footprint stamp for tf lookup
• Removed footprint_timeout from FootprintSubscriber
• Updated FootprintSubscriber API and added doxygen description to getFootprintRaw and getFootprintInRobotFrame methods

Description of documentation updates required from your changes

• Removed global_frame parameter from smoother server, so need to remove that from default configs and documentation page

---

Future work that may be required in bullet points

• Check for other places where tf lookup has possibly been used improperly

For Maintainers:

• Check that any new parameters added are updated in navigation.ros.org
• Check that any significant change is added to the migration guide
• Check that any new functions have Doxygen added
• Check that any new features have test coverage
• Check that any new plugins is added to the plugins page
• If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

[SteveMacenski]

You have tests failing, in particular the collision test isn't a flaky test. If you pull in main as well, I recently fixed a bunch of flaky tests and at least release builds should be totally green now.

[SteveMacenski]

I’ll look at the sp. comments in the morning, but did want to mention quickly that tests are failing so something’s not quite right. We’d need to fix that before merging regardless.

Also, pull in main or rebase, the release tests should be totally green. If there’s a flaky debug test that wouldnt be concerning. I’m working through that issue this week.

[mergify]

@robotechvision, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

[mergify]

@robotechvision, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

[robotechvision]

Could you please have a look at the test results? I fixed the tests and they work on my machine but some infrastructure problems are reported here and an absolutely unrelated fibonacci test is causing CI crash

[SteveMacenski]

These changes look good to me. The only subjective question I'd have is whether we should adda function into the footprint subscriber for transforming to the pose as well, rather than in the costmap collision checker, so that a user of the footprint subscriber could get the raw, unoriented in base frame, or transformed footprint.

[SteveMacenski]

Remaining things are just adding a migration guide section update. I believe with removing footprint_tolerance and renaming the TF tolerance, there are no other parameter updates so no need to update any of the guides.

[afrixs]

Regarding the question, it could be a nice bonus to provide such a function, but I personally think it doesn't need to be under the responsibility of the subscriber, what will be done next with the footprint. Especially when it would save just one simple line of code. I updated transformFootprint method so that it can take the same variable as input and output, making this subsequent transform simpler and less error-prone.

[SteveMacenski]

Last question, then good to be merged

[SteveMacenski]

I think fetch should be default to true like the other functions above so default behavior gets the latest footprint. Maybe also rename to fetch_latest_footprint or something more descriptive as well.

An inline code comment in the collision checker regarding these fetch and fetch_costmap_and_footprint would be nice to tell users why this is there (so if looping over many poses, using the same costmap / footprint since the costmap specifically would incur a pretty gnarly copy if working on the global costmap).

[SteveMacenski]

Basically, just want this context to be made clear to future readers

[mergify]

@robotechvision, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

[mergify]

@robotechvision, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

[SteveMacenski]

Sorry about Circle, maybe some update came on the servers, I'm not sure why just the debug builds would be failing. If it persists, I'll look into it.

I proposed a change above, I'd just merge it in and then merge in the PR, but wanted you to approve it. If you approve it and commit the suggestion, I'll merge this PR on Monday.

[SteveMacenski]

Ugh, I really don't mean this to be the never ending PR but it failed a linting check

Diff with 7 lines
--- include/nav2_costmap_2d/costmap_topic_collision_checker.hpp
+++ include/nav2_costmap_2d/costmap_topic_collision_checker.hpp.uncrustify
@@ -86 +86,3 @@
-  Footprint getFootprint(const geometry_msgs::msg::Pose2D & pose, bool fetch_latest_footprint = true);
+  Footprint getFootprint(
+    const geometry_msgs::msg::Pose2D & pose,
+    bool fetch_latest_footprint = true);

If you haven't fixed it by the time I get to this Monday morning during work hours, I'll just merge it and quickly submit the patch to fix. I don't want to have this be hung up any longer for something so small.

[SteveMacenski]

Thanks for the patch, this was a subtle issue!