[ROS 2][grid_map_core] Update ROS 2 branch with the latest changes from master

[kjalloul-anybotics]

Changes to grid_map_core

• Added latest fixes and test cases from master.
• Bumped package version to 2.4.0.

[wep21]

@kjalloul-anybotics rebase, please

[Ryanf55]

Thanks for the contribution, nice to see some support for bringing the ROS 2 branch up to date!

As it stands, you've bundled a number of unrelated changes. Can you instead propose PR's that target things individually so it's more reviewable? Also, bumping the version can be done as it's own PR.

In addition to the changes outlined, you have

• Replaced ifdefs with pragma_once
• Fixed spelling
• Added a union object for the color
• Added test failure messages that don't change the test logic
• Changed CubicInterpolation's underflow behavior
• Cleaned up return value syntax
• Switched to using std::all_of instead of manual iteration
• Switched which exception is thrown in GridMap without updating the public API - this is API breaking and we cannot backport this
  ... and more...

That's a lot of risk, with no linkage to the original commit history or PR's. Perhaps there's a better strategy we can come up with to make this safer?

[kjalloul-anybotics]

Thanks for the feedback!

Yes in the beginning I only needed one new function in grid_map_cv, but then while I was at it, the idea was to bring it and its upstream dependency grid_map_core up to date with master.

But you're right it definitely can be broken down into smaller PRs to keep track of where the changes came from.

I traced back the origin of most changes to these commits:

• std::all_of changes, return value syntax changes, and other fixes related to formatting from b7293f5
• Typos, updated tests, adding namespace to tests, and more syntax improvements from d6f0912
• Adding Color union, removing a couple functions, and fixing compiler warnings from 6ddff74
• Clipping point fix and new test cases from f1344bd
• Visibility cleanup from 61972c8
• Transformed map fix + tests from f0dd780
• Submap iterator speedup + tests from c8b1057

Do you think it's a good idea to create separate PRs for each of them? Once grid_map_core is updated, I can do the same for grid_map_cv.

And apparently pragma was replaced with ifndef during the ROS 2 port and not the other way around, same with the switch cases to if conditions, so I'll revert these changes.

[Ryanf55]

Awesome, thanks for putting the list together!

yes, if it's easy enough, separate PR's are preferred, with a link to the ROS 1 PR.

The change to ifdefs was probably intentional, and no one has complained about it, so I'd personally vote we just not touch it.
Unless we have a reason to diverge, I would prefer the conventions in rclcpp:
https://github.com/ros2/rclcpp/blob/e64627004f4265560addf3532510a83602be5aef/rclcpp/include/rclcpp/client.hpp#L16