Document alternative for declaring an optional parameter

[jacobperron]

While we could conditionally declare a parameter by using a second
parameter, we can also just catch the expected exception in the
case that no parameter override is provided.

I think this makes for a better user experience.

[wjwwood]

I think to be equivalent to the above, wouldn't it need to be like this:

Suggested change

	Or, wrap the declaration in a try-catch:
	```cpp
	try {
	node->declare_parameter<int64_t>("param_that_is_optional");
	} catch (const rclcpp::exceptions::NoParameterOverrideProvided &) {
	// a parameter value was not provided
	}
	```
	Or, wrap the declaration in a try-catch:
	```cpp
	auto mode = node->declare_parameter("mode", "modeA");
	try {
	node->declare_parameter<int64_t>("param_that_is_optional");
	} catch (const rclcpp::exceptions::NoParameterOverrideProvided &) {
	// a parameter value was not provided
	if (mode == "modeB") {
	// parameter and parameter override was required
	throw;
	}
	}
	```

What you've described here is more like "conditionally declare a parameter if an override is given", which is like a subset of

rclcpp/rclcpp/include/rclcpp/node_options.hpp

Lines 329 to 343 in 7bfda87

	/// Set the automatically_declare_parameters_from_overrides, return this.
	/**
	* If true, automatically iterate through the node's parameter overrides and
	* implicitly declare any that have not already been declared.
	* Otherwise, parameters passed to the node's parameter_overrides, and/or the
	* global arguments (e.g. parameter overrides from a YAML file), which are
	* not explicitly declared will not appear on the node at all, even if
	* `allow_undeclared_parameters` is true.
	* Already declared parameters will not be re-declared, and parameters
	* declared in this way will use the default constructed ParameterDescriptor.
	*/
	RCLCPP_PUBLIC
	NodeOptions &
	automatically_declare_parameters_from_overrides(
	bool automatically_declare_parameters_from_overrides);

.

Also, I'm kind of concerned with the conditionally declared parameters. I'm not saying they shouldn't be allowed, but if I were making a system, I'd prefer the parameters to be the same no matter what, and if one was not provided (and that was ok) it should be "not set" or something. I'm thinking about "dumping" parameters from a node in various scenarios. It's maybe undesirable to have different number and order of parameters in the dump depending on the circumstances, because then if a parameter is missing from the dump, it's hard to know if it was not specified or if the dump maybe came from a difference version of the program that didn't have the parameter yet/any longer. But that's just a architectural decision for the developer of the node.

[jacobperron]

What you've described here is more like "conditionally declare a parameter if an override is given"

Right, this is exactly what I was trying to do. For situations where we want an optional parameter, but perhaps there are no default values that make sense. I guess what you're suggesting is that the author should try to avoid putting themselves in this situation.

This came up because I've already hit several places in the wild where code is trying to declare optional parameters, for example: cra-ros-pkg/robot_localization#631 (comment)

It would be nice if declare_parameter wouldn't throw, but we made that compromise for technical reasons, IIRC.

[jacobperron]

Maybe what we need is a representation for a value that is "not set"; currently, we only allow the type to be "not set".

[wjwwood]

Yeah, I think this is essentially what @ivanpauno was talking about with "uninitialized parameters". It can have a type, say int64_t, but the value is either an int64_t or uninitialized.

[ivanpauno]

I like @wjwwood suggestion here.
If not, the try/catch block isn't completely equivalent.

Also, I'm kind of concerned with the conditionally declared parameters. I'm not saying they shouldn't be allowed, but if I were making a system, I'd prefer the parameters to be the same no matter what, and if one was not provided (and that was ok) it should be "not set" or something. I'm thinking about "dumping" parameters from a node in various scenarios.

That sounds fair.

[jacobperron]

Somewhat related, I ran into another scenario where I have a generated list of names and types for which I want to declare parameters. The problem is that I don't want to require users to provide overrides at runtime, rather I expect values may be provided during runtime (e.g. with ros2 param set), well after the node starts. The only workaround I found was to conditionally call declare_parameter based on the parameter type, e.g.:

void declare_parameter_with_default(const std::string & name, const rclcpp::ParameterType & type)
{
  switch (type) {
    case rclcpp::ParameterType::PARAMETER_INTEGER:
      parameters_interface_->declare_parameter(name, rclcpp::ParameterValue(0));
      break;
    case rclcpp::ParameterType::PARAMETER_DOUBLE:
      parameters_interface_->declare_parameter(name, rclcpp::ParameterValue(0.0));
      break;
    case rclcpp::ParameterType::PARAMETER_STRING:
      parameters_interface_->declare_parameter(name, rclcpp::ParameterValue(""));
      break;
    case rclcpp::ParameterType::PARAMETER_INTEGER_ARRAY:
      parameters_interface_->declare_parameter(name, rclcpp::ParameterValue(std::vector<int>()));
      break;
    case rclcpp::ParameterType::PARAMETER_DOUBLE_ARRAY:
      parameters_interface_->declare_parameter(
        name, rclcpp::ParameterValue(std::vector<double>()));
      break;
    default:
      throw std::runtime_error("Unexpected parameter type when declaring parameters");
  }   
}

I would rather replace the above code with:

parameters_interface_->declare_parameter(name, type);

but it throws an exception if the user does not provide an override.

[wjwwood]

That use case makes sense to me @jacobperron

[ivanpauno]

Yes, that last use case sounds like a good case for having an "uninitialized" state for statically typed parameters.

[jacobperron]

I've opened a ticket for allowing uninitialized parameters: #1649

[jacobperron]

In retrospect, I agree that we shouldn't encourage the conditional declaration of parameters, so I'm going to close this ticket. I'd much rather see #1649 resolved, supporting "uninitialized" parameters.