Bringing back dynamic parameters: Costmap2D

[bpwilcox]

This PR brings the ParameterEventsSubscriber class into the nav2_utils and re-enables run-time configuration of parameters. As a first re-integration step, this PR adds parity with ROS1 dynamic parameters for the costmap2d package.

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	#956
Primary OS tested on	Ubuntu
Robotic platform tested on	turtlebot3 simulation

---

Description of contribution in a few bullet points

• Adds ParameterEventsSusbcriber parameter event callbacks
• adds parameter callbacks for costmap layers
• adds event_callback for Costmap2DROS

Future work that may be required in bullet points

• re-enable dynamic callbacks in other packages, namely nav2_amcl and nav2_dwb_controller
• re-access usage of dynamic paramters throughout the stack
• If my PR upstream (Add ParameterEventsSubscriber class ros2/rclcpp#829) is merged, then we can remove/replace the class from nav2_utils

[codecov]

Codecov Report

Merging #1370 into master will decrease coverage by 16.01%.
The diff coverage is 1.7%.

@@             Coverage Diff             @@
##           master    #1370       +/-   ##
===========================================
- Coverage   40.73%   24.71%   -16.02%     
===========================================
  Files         233      235        +2     
  Lines       11185    11477      +292     
  Branches     4822     3679     -1143     
===========================================
- Hits         4556     2837     -1719     
- Misses       3452     6709     +3257     
+ Partials     3177     1931     -1246

Flag	Coverage Δ
#project	24.71% <1.7%> (-16.02%)	⬇️

Impacted Files	Coverage Δ
nav2_costmap_2d/include/nav2_costmap_2d/layer.hpp	54.54% <ø> (-9.1%)	⬇️
...map_2d/include/nav2_costmap_2d/inflation_layer.hpp	82.75% <ø> (-10.35%)	⬇️
...tmap_2d/include/nav2_costmap_2d/costmap_2d_ros.hpp	0% <ø> (-63.64%)	⬇️
...tmap_2d/include/nav2_costmap_2d/obstacle_layer.hpp	100% <ø> (ø)	⬆️
...costmap_2d/include/nav2_costmap_2d/voxel_layer.hpp	0% <ø> (-8%)	⬇️
.../include/nav2_util/parameter_events_subscriber.hpp	0% <0%> (ø)
nav2_costmap_2d/plugins/voxel_layer.cpp	0.38% <0%> (-23.82%)	⬇️
nav2_util/src/parameter_events_subscriber.cpp	0% <0%> (ø)
nav2_costmap_2d/src/costmap_2d_ros.cpp	0% <0%> (-28.18%)	⬇️
nav2_costmap_2d/plugins/static_layer.cpp	31.21% <10%> (-3.76%)	⬇️
... and 105 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update abe5167...4724451. Read the comment docs.

[SteveMacenski]

Please file a separate ticket for observation obstacle/voxel layer reconfigure.

Can you also validate this works with rqt reconfigure? That’s the primary client of this work so I want to be sure it works as expected.

[bpwilcox]

@SteveMacenski from what I can tell on my machine, rqt_reconfigure is having trouble loading up all the parameters from all the nodes, gets stuck and never comes up for me to try changing parameters with the full stack.

[SteveMacenski]

Can you try with just 1 node running?

[bpwilcox]

@SteveMacenski I was once able to get it to work on a test node (non-navstack related), but it has been very buggy, many freezes or crashes since. Have you played with the plugin yourself recently?

[SteveMacenski]

No, reconfigure is something I use once when setting up a node and then usually never again

[SteveMacenski]

I'm not blocking over that, but it would be nice to know that this works via that as a general rule

[bpwilcox]

Seemingly the reconfigure tool is not very stable currently, so perhaps we can come back to this issue at some point later,

[bpwilcox]

From a discussion today, there was the concern of whether dynamic parameters would/should be updated in a real production system. If by default as in ROS1, dynamic parameters are always allowed, it would be up to the user to know not to change them for their use case. An additional measure would be to set parameters as read-only in the their descriptor field, which at the moment is only specified in code, but with my PRs upstream to parse descriptors in yaml, would be configurable at launch. If we want to allow the parameters to be set on the node, but simply not to take effect when in the active state of the lifecycle node, we could either define a mode in which parameters callbacks are allowed (i.e in active or inactive state), or a parameter flag which would turn off dynamic callbacks entirely, still allowing the node to be re-configured via the lifecycle reset (cleanup->configure).

I believe we arrived at thinking we should allow the flag to skip all parameter callbacks if the user doesn't desire. Also, moving more code from the event callback to be smaller parameter updates and only doing extra work when related to the parameter change. I also realized that the callback handles should be cleared when the node is cleaned up, so I will add that.

[SteveMacenski]

I don’t think you need to go crazy to stop people from doing things like this. People that value the lifecycle elements are going to understand this break some of the benefits.

People dont really use dyn reconfigure in production (using to change things in usual modes) or when they do I think we all understand its not good practice and is a bit hacky.

I think making the capability, thats analog to dynamic reconfigure, is fine. I wouldn’t overthink it.

[mjeronimo]

Responses to parameter changes should be more granular. That is, only data structures/resources that depend on the parameter changes should be re-created and/or re-initialized.

[bpwilcox]

I agree, but I think we can make that a separate PR.