Modify Parameters On Set Parameter Callback

[threeal]

Feature Request

Feature Description

Currently the rclcpp::Node::add_on_set_parameter_callback() only support callback of rcl_interfaces::msg::SetParametersResult callback(const std::vector<rclcpp::Parameter> &parameters). As explained in the add_on_set_parameters_callback() documentation:

The callback signature is designed to allow handling of any of the above set_parameter* or declare_parameter* methods, and so it takes a const reference to a vector of parameters to be set, and returns an instance of rcl_interfaces::msg::SetParametersResult to indicate whether or not the parameter should be set or not, and if not why.

The problem with const reference is we cannot modify the value of the parameters to be set inside the on set parameter callback. And the bad things, we also couldn't call set_parameter() inside the callback as stated in the documentation

The callback may introspect other already set parameters (by calling any of the {get,list,describe}_parameter() methods), but may not modify other parameters (by calling any of the {set,declare}_parameter() methods) or modify the registered callback itself (by calling the add_on_set_parameters_callback() method). If a callback tries to do any of the latter things, rclcpp::exceptions::ParameterModifiedInCallbackException will be thrown.

And by seeing the example in the documentation, we could conclude that the on set parameter callback could only filter an invalid parameter by rejecting it.

rcl_interfaces::msg::SetParametersResult
my_callback(const std::vector<rclcpp::Parameter> & parameters)
{
  rcl_interfaces::msg::SetParametersResult result;
  result.successful = true;
  for (const auto & parameter : parameters) {
    if (!some_condition) {
      result.successful = false;
      result.reason = "the reason it could not be allowed";
    }
  }
  return result;
}

And why would someone want to modify the parameters during on set parameter callback? Considered the following scenario, i have a parameter that should only accept the double value of 0.0 to 100.0. if i set the parameter with double value between 0.0 to 100.0, then it would be fine. but what if i accidently set the parameter to be 101.0, using the current on_set_parameter_callback() behavior, then i could only reject it. wouldn't it be better if i could accept it in condition that the parameter value is clamped to 100.0? or what if i accidently set the parameter using an integer value of 50? when i accept it, the parameter would simply changed from double to integer, and it would be considered bad. But i shouldn't reject it either. Wouldn't it be better if i could accept it in condition that the parameter type to be casted to double value?.

Implementation Considerations

Add a new callback type for rclcpp::Node::add_on_set_parameter_callback() with the type of rcl_interfaces::msg::SetParametersResult callback(std::vector<rclcpp::Parameter> &parameters).

For note, I'm already tried to solve the problem with const casting the parameters to std::vector<rclcpp::Parameter> &. It worked, but kinda be a tricky solution and a bad habit as the parameters was implicitly stated to be constant.

[clalancette]

But considered the following scenario, i have a parameter that only accept the double value of 0.0 to 100.0. if i set the parameter with double value between 0.0 to 100.0, then it would be fine. but what if i set the parameter to be 101.0, using the current on_set_parameter_callback() behavior, then i could only reject it. wouldn't it be better if i could accept it in condition that the parameter value is clamped to 100.0?

I'm not following what feature you are looking for here. Can you explain more?

With the current callback, it is entirely possible to clamp a value as you set it. You just use a std::clamp as you set the value in the callback. As for setting the range between 0 and 100.0; to me, it seems non-sensical to set a range, and then to ignore that range and clamp it. That is, if you want to accept any value and clamp it, then don't set a range.

Anyway, more information here on what you are trying to do (including a code example) would be most helpful.

[threeal]

@clalancette i updated my feature description. so basically, there's no way that allows a node to modify the parameters that will be set during the on_set_parameter_callback. Even a std::clamp inside the on_set_parameter_callback won't work as the parameter itself is immutable. the only things that could be done during on_set_parameter_callback is rejecting an invalid parameter.

[fujitatomoya]

i have a parameter that should only accept the double value of 0.0 to 100.0, but what if i accidently set the parameter to be 101.0.

i guess that is the setter's responsibility, since setter sets the parameter which is out of range? I would add clipping what makes sure the parameter will be set in the rage of 0.0 to 100.0 in the setter.

[clalancette]

Even a std::clamp inside the on_set_parameter_callback won't work as the parameter itself is immutable. the only things that could be done during on_set_parameter_callback is rejecting an invalid parameter.

Ah, I see where the disconnect is now. When I've used this in the past, I've always had a separate member variable internal to the class that I set. That is, my parameterCallback() looks something like:

rcl_interfaces::msg::SetParametersResult
paramCb(const std::vector<rclcpp::Parameter> & parameters)
{
  rcl_interfaces::msg::SetParametersResult result;
  result.successful = true;
  for (const auto & param : parameters) {
    if (param.name() == 'foo') {
      my_internal_foo_ = std::clamp(param.get(), 0.0, 100.0);
    }
  }
  return result;
}

Then later code uses my_internal_foo_:

void doThingLater()
{
    doThing(my_internal_foo_);
}

But I think what you are trying to do is something more like:

rcl_interfaces::msg::SetParametersResult
paramCb(std::vector<rclcpp::Parameter> & parameters)
{
  rcl_interfaces::msg::SetParametersResult result;
  result.successful = true;
  for (const auto & param : parameters) {
    if (param.name() == 'foo') {
     param.value = std::clamp(param.get(), 0.0, 100.0);
    }
  }
  return result;
}

And then later:

void doThingLater()
{
    doThing(this->get_parameter("foo"));
}

@threeal Can you confirm that this is the use case you are trying to support?

[ivanpauno]

@threeal friendly ping

[threeal]

I'm sorry, I haven't reached this thread for a while. Yes that's exactly what i means, as described by @clalancette.

[clalancette]

Right, thanks for the feedback. There are two different things here:

1. I think we have a long-term goal of being able to automatically clamp values for parameters based on the parameter description. That way you wouldn't even have to write a parameter callback to accomplish this. I don't think anyone is currently working on this, so help here would be appreciated. It should be pretty straightforward; we already have the ability to specify ranges in the parameter descriptor, we just need to enforce it when it is changed.
2. For the more general case of being able to modify the parameters while in the parameter callback, what you propose would indeed work. The problem is that I don't think we can have function signatures with both const and non-const (C++ doesn't know which one to call). That means that we'd have to change the signature of the parameter callbacks again.

My suggestion for now is to implement the first one. Let me know if you need any help with it, and I can provide a few more pointers.

[berndpfrommer]

Using a validator before updating the parameters is all nice and well if validation can be done beforehand. But I'm running into cases where a driver will adjust parameters that have been set, based on rules that are either too complex to reasonably encode, or are not documented at all. For instance the frame rate of a camera depends on the exposure time, the model-dependent camera dead time, the link bandwidth, etc etc. In this case, how do you provide feedback that the parameter has been adjusted (e.g. you tried setting the frame rate to 40fps, but the driver knocked it back down to 33.7fps)? I have another case of a driver for an experimental camera that is even more complex. Basically there is no way to write a good validator.

[threeal]

Using a validator before updating the parameters is all nice and well if validation can be done beforehand. But I'm running into cases where a driver will adjust parameters that have been set, based on rules that are either too complex to reasonably encode, or are not documented at all. For instance the frame rate of a camera depends on the exposure time, the model-dependent camera dead time, the link bandwidth, etc etc. In this case, how do you provide feedback that the parameter has been adjusted (e.g. you tried setting the frame rate to 40fps, but the driver knocked it back down to 33.7fps)? I have another case of a driver for an experimental camera that is even more complex. Basically there is no way to write a good validator.

check my note, basically we could use rclcpp::Node::add_on_set_parameter_callback() to create a validator, and the hack is constant casting the parameter, and modify the value there

[berndpfrommer]

Const casting the parameter is obviously not a permanent solution. Neither is hacking it the way I currently do by starting a timer inside the parameter callback handler to defer the modification of the parameter:

          changeTimer_ = rclcpp::create_timer(
            this, this->get_clock(), rclcpp::Duration::from_nanoseconds(10000000), [=]() {
              changeTimer_->cancel();
              this->set_parameter(rclcpp::Parameter(p.get_name(), newVal));
            });

[clalancette]

Using a validator before updating the parameters is all nice and well if validation can be done beforehand. But I'm running into cases where a driver will adjust parameters that have been set, based on rules that are either too complex to reasonably encode, or are not documented at all. For instance the frame rate of a camera depends on the exposure time, the model-dependent camera dead time, the link bandwidth, etc etc. In this case, how do you provide feedback that the parameter has been adjusted (e.g. you tried setting the frame rate to 40fps, but the driver knocked it back down to 33.7fps)? I have another case of a driver for an experimental camera that is even more complex. Basically there is no way to write a good validator.

So I would say that this case is probably beyond the scope of what a set_parameter_callback() can do (at least, in the current design). What I would consider doing here is the following:

1. Use an set_parameter_callback() to validate that the parameter is within any reasonable bounds. In the case for the camera FPS, if the hardware only supports up to 100 Hz, but the parameter input was 1000 Hz (or negative), reject it at this stage. Accept everything else.
2. Use an on_parameter_event callback to actually make the changes to the hardware, like in https://github.com/ros/robot_state_publisher/blob/b6fbbd0a821c14d6ea34f83bcab93c77358caeef/src/robot_state_publisher.cpp#L159-L161 .
3. The next bit depends on how the hardware behaves. If you get immediate feedback from the hardware that the change was lower than expected, call set_parameter again right within the on_parameter_event callback. If the hardware doesn't tell you the speed until some point later, use some custom logic (either a timer or another event from the hardware) to check and update the parameter.

I think the above should give you the behavior you want. It is a somewhat clunky approach, but at the moment I don't have much better idea.

[berndpfrommer]

@clalancette thanks, works for me! I did not know about the asynchronous on_parameter_event. Note: in this particular case the driver SDK immediately returns the adjusted value which simplifies the logic.

[clalancette]

Given the workaround worked, and the fact that we added in pre- and post- parameter callbacks on #1947 , I'm going to close this one out. If you are still having issues, please feel free to open a new issue.