No need to spam the logs

[Timple]

This pushes out all other logs. I think once every 5 seconds is enough to catch the eye.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
...map_2d/include/nav2_costmap_2d/layered_costmap.hpp	100.00% <ø> (ø)
nav2_costmap_2d/src/costmap_2d_ros.cpp	86.54% <100.00%> (-0.27%)	⬇️
nav2_costmap_2d/src/layered_costmap.cpp	96.63% <100.00%> (+0.17%)	⬆️

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[SteveMacenski]

I generally agree with you, but I don't love exposing ROS at increasingly lower levels than already done so. There's only a single log that is using the throttle and its in a situation that should, practically speaking, never happen except in a poor situation.

Can you provide some motivating feedback about why you're seeing so much logging like this that a change like this is necessary -- and the cause of the error cannot be fixed?

Sorry for the delay - ROSCon + vacation and all that jazz 😄

[SteveMacenski]

I don't think there's a sane default for the clock type like this. I think this needs to always be a real clock if a clock is used. That's an argument - to me - to change the constructor of LayeredCostmap to have the clock/logger fields at the end in the existing constructor rather than adding a new one. If we're adding it, make it strictly required since there's no sane workaround.

[Timple]

I generally agree with you, but I don't love exposing ROS at increasingly lower levels than already done so. There's only a single log that is using the throttle

Yes, that was my original feeling as well. So we might as well leave this PR to it's first commit?

and the cause of the error cannot be fixed?

It can and it's already fixed. But the terminal output was scrolling up fast so I had to scroll a lot to get to the rootcause 😄

I don't feel strongly about this PR. But in general I don't like log spams so I fixed it. We can

A) Drag in the logger and the clock.
B) Keep it at the first commit and simply construct the clock only in rare scenarios.
C) Drop this PR alltogether.

[SteveMacenski]

B) Keep it at the first commit and simply construct the clock only in rare scenarios.

I like this one. Lets just do this. With my long term planning surrounding costmaps, I don't want to invest too much time in Costmap2D but I also don't want to muddle it up more than it already is 😆

[mergify]

@Timple, 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).