Is this what happens currently when you chain parameters like this ? (Should behave like this ideally I believe)

param A (set successfully) --> param B (fails to set) --> param C (should not be triggered) 

Is param B and param C being set inside post callback?

Yup, So I meant A's callback sets B, B's callback sets C.

I will check this to confirm the behaviour.

I checked, this is indeed the behaviour right now, so should be good.

Some related discussion here : #1190, #609

To recap from our discussion at the ROS 2 meeting:

• Our current plan is to introduce a "pre set parameters" callback, that can return a modified list of parameters that is then forwarded to the current "on set parameters" callback for validation. This should satisfy the use-case of updating additional parameters conditioned on changes to a parameter.
• The post callback is still valuable as a place to cause side-effects based on parameter changes, but we will avoid subverting the parameter recursion lock (which we no longer need since we can use the pre-callback to request changes to parameters based on another).

Not sure but should we do something about add_pre_set_parameters_callback, add_on_set_parameters_callback, add_post_set_parameters_callback having almost similar code except that they are using different callback containers.

One more thing which we should probably address is that since the pre_set_parameter_callback will be mainly used to modify the rclcpp::Parameter list, this might result in a different behaviour where the set_parameters_atomically should have been called non-atomically but is being called atomically now.

We can probably do something about this by not calling the registered pre_set_parameter_callback inside the set_parameters_atomically but outside it.

I have made changes to the code based on review suggestions. I have also updated documentation for existing Node API's based on pre and post callbacks and can use a review for that specific commit d0bc26b

Thanks!

Please trigger a CI job from ci.ros2.org and make sure it is green before merging.

CI :
build : --packages-above-and-dependencies rclcpp
test : --packages-above rclcpp

• Linux
• Linux-aarch64
• Windows

The same Windows test failure occurred on the most recent nightly: https://ci.ros2.org/view/nightly/job/nightly_win_rep/lastBuild/testReport/junit/launch_testing_examples.launch_testing_examples/check_multiple_nodes_launch_test/launch_testing_examples_check_multiple_nodes_launch_test/