Setting a parameter can create a mismatch in parameter value between the internal value and the ros2 param value

[JasperTan97]

When setting a parameter, the on_validate_parameter_callback is called, and sometimes the value of the parameter is changed (for example, if the candidate value is out of range) instead of rejected. However, if we set the parameter via the ROS2 cli, ROS2 does not allow a parameter to be set or changed during the set parameter callback, which includes on_validate_parameter_callback. Hence, the internal value of our parameter may get updated via the logic in on_validate_parameter_callback, but the value of the ROS2 parameter is left as what has been set.

From testing, both using the ros2 param cli and the AICA API (via localhost:5000) have the same issue. There needs to be a solution to re-synchronise both parameter maps.

[JasperTan97]

@domire8

[domire8]

Yeah this is an interesting one @eeberhard. It is indeed a problem that you can modify the value of the shared parameter object that one gets on_validate_parameter_callback which as a consequence will have an effect on the value of the internal parameter object value, but NOT on the value of the parameter on the ROS 2 server.

"Re-synchronize" the parameters is one way, even though I wouldn't see an efficient way to do that right now. A more correct way would maybe be to prohibit a change to the shared parameter object on_validate_parameter_callback. Should the signature of that function change to const std::shared_ptr<CONST state_representation::ParameterInterface>&?

[eeberhard]

Interesting. Let me just summarize again the intended behavior of the parameter validation callback:

During parameter validation, we have a kind of "parameter candidate" which comes from the ROS 2 interface which is passed to on_validate_parameter_callback. This function is allowed to modify the parameter candidate and "lock it in" by returning true, else discard the candidate by returning false. The intention here was to automatically allow numeric parameters to be constrained by limits, string parameters to be sanitized, etc for a smoother user experience.

Now to check that I understand the issue correctly; what I expect is that modifying the parameter candidate in the callback does modify the actual ROS 2 parameter as expected when setting a parameter through a parameter client from another node (for example the Dynamic State Engine in the AICA application framework). You are saying it does not modify the actual ROS 2 parameter when setting a parameter through the ROS 2 CLI.

However, if we set the parameter via the ROS2 cli, ROS2 does not allow a parameter to be set or changed during the set parameter callback

I don't fully understand this; how can ROS 2 CLI set any parameter in that case? Do you just mean it can't be changed (read-only)?

A more correct way would maybe be to prohibit a change to the shared parameter object

Are you implying to remove the ability to modify parameter candidates entirely? For me that's a last resort, since I think this feature is otherwise a useful and powerful part of our abstraction.

I think it would be better to first understand why there is a difference between setting parameters through the CLI vs through other parameter clients (if there even is a difference at all; perhaps this problem has been present no matter where the parameter change originates from).

[domire8]

Are you implying to remove the ability to modify parameter candidates entirely? For me that's a last resort, since I think this feature is otherwise a useful and powerful part of our abstraction.

This is useful, but it is currently not reflected on the ros2 parameter server. To see that we have to look here. As you mention, we can modify the parameter candidate and change the value of the internal parameter like that. However, these changes are never translated back to ROS, so we have a mismatch between the ROS parameter server and the internal parameter map. Since the argument of the callback function is a const reference to a vector, we will not be able to reflect a changed parameter candidate back onto the ROS server, hence my suggestion to remove that option since it's very misleading.

I think it would be better to first understand why there is a difference between setting parameters through the CLI vs through other parameter clients (if there even is a difference at all; perhaps this problem has been present no matter where the parameter change originates from).

There isn't. The confusion came from the fact that internally the component had the modified value, but not on the parameter server.

However, if we set the parameter via the ROS2 cli, ROS2 does not allow a parameter to be set or changed during the set parameter callback

This has not much to do with the CLI but what one might try to avoid above issues is to use set_parameter_value(...) from within the on_validate_parameter_callback which is going to fail because set_parameter_value triggers the on_set_parameter_callback so you get an error that you can't call the callback from the callback...

[domire8]

Looking at the node parameters interface we can see that there are three possible callbacks on set of a parameter:

struct PreSetParametersCallbackHandle
{
  RCLCPP_SMART_PTR_DEFINITIONS(PreSetParametersCallbackHandle)

  using PreSetParametersCallbackType =
    std::function<void (std::vector<rclcpp::Parameter> &)>;

  PreSetParametersCallbackType callback;
};

struct OnSetParametersCallbackHandle
{
  RCLCPP_SMART_PTR_DEFINITIONS(OnSetParametersCallbackHandle)

  using OnSetParametersCallbackType =
    std::function<
    rcl_interfaces::msg::SetParametersResult(
      const std::vector<rclcpp::Parameter> &)>;
  using OnParametersSetCallbackType [[deprecated("use OnSetParametersCallbackType instead")]] =
    OnSetParametersCallbackType;

  OnSetParametersCallbackType callback;
};

struct PostSetParametersCallbackHandle
{
  RCLCPP_SMART_PTR_DEFINITIONS(PostSetParametersCallbackHandle)

  using PostSetParametersCallbackType =
    std::function<void (const std::vector<rclcpp::Parameter> &)>;

  PostSetParametersCallbackType callback;
};

We're using the OnSetParametersCallbackType now, that has const std::vector<rclcpp::Parameter> & as argument. We see that the PreSetParametersCallbackType has a mutable argument std::vector<rclcpp::Parameter> &. However, there is no return value, so we couldn't reject parameters just by using this callback. I was somehow hoping the PostSetParametersCallbackType could be an easy fix but its also const so I'm back to square 1

[eeberhard]

However, there is no return value, so we couldn't reject parameters just by using this callback

Couldn't we use our internal store of the parameter to overwrite the candidate parameter with the "old" value to functionally reject the change?