rclcpp icon indicating copy to clipboard operation
rclcpp copied to clipboard

Parameter doesn't allow setting of name or value

Open gerkey opened this issue 9 years ago • 5 comments

The ParameterVariant class requires the name and value (and thus type) of the variable to be passed to the constructor. To make it easier to use, we should add set_name() and set_value() methods.

gerkey avatar Jul 08 '16 22:07 gerkey

Related enough to be mentioned here: ParameterVariant should allow setting from a rcl_interfaces::msg::ParameterValue.

gerkey avatar Jul 09 '16 00:07 gerkey

I changed the title since ParameterVariant was split into Parameter and ParameterValue in #481 and Parameter still doesn't have set_name() and set_value() methods.

sloretz avatar Jul 05 '18 18:07 sloretz

We are using member-based access in ROS message classes. So I don't think we should add a method for these two fields.

In C++ we generally don't have constructors with arguments (since the order would be problematic in case of future changes to the message). So the user has to set all fields after creating an instance.

So I am not sure what the scope of this ticket is. If it is just about adding methods to set name and value (over setting them as members) I would suggest to close it.

dirk-thomas avatar Jul 05 '18 19:07 dirk-thomas

@dirk-thomas currently there is no way to change the name or value of an instance of rclcpp::Parameter. Are you saying you would rather see its members be changed from private to public instead of adding mutator methods?

sloretz avatar Jul 05 '18 20:07 sloretz

It wasn't clear from the description that this ticket is referring to the rclcpp class (sure, it is filled in this repo :wink:). I was assuming it was referring to the rcl_interfaces with that name (since that one was explicitly mentioned in the second comment). For the C++ class it would make sense to add setters.

We can either add those or just close this ticket since this is more a nice to have rather than preventing the user to use the class.

dirk-thomas avatar Jul 05 '18 21:07 dirk-thomas