Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameter doesn't allow setting of name or value #238

Open
gerkey opened this issue Jul 8, 2016 · 5 comments
Open

Parameter doesn't allow setting of name or value #238

gerkey opened this issue Jul 8, 2016 · 5 comments
Labels
backlog enhancement New feature or request

Comments

@gerkey
Copy link
Member

gerkey commented Jul 8, 2016

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
Copy link
Member Author

gerkey commented Jul 9, 2016

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

@sloretz sloretz changed the title ParameterVariant doesn't allow setting of name or value Parameter doesn't allow setting of name or value Jul 5, 2018
@sloretz
Copy link
Contributor

sloretz commented Jul 5, 2018

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.

@dirk-thomas
Copy link
Member

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 dirk-thomas added the more-information-needed Further information is required label Jul 5, 2018
@sloretz
Copy link
Contributor

sloretz commented Jul 5, 2018

@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?

@dirk-thomas
Copy link
Member

dirk-thomas commented Jul 5, 2018

It wasn't clear from the description that this ticket is referring to the rclcpp class (sure, it is filled in this repo 😉). 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 dirk-thomas removed the more-information-needed Further information is required label Jul 5, 2018
nnmm pushed a commit to ApexAI/rclcpp that referenced this issue Jul 9, 2022
* remove old memory tools

* updates to use new memory_tools from osrf_testing_tools_cpp

* fixup new test

* fix lambda captures for Windows

* uncrustify fix

* extra_test_env -> rmw_implementation_env_var

* Stray extra_test_libraries

* Use default rmw impl outside of for_each_rmw_implementation loop

* style fixup

* fix typo
@clalancette clalancette added enhancement New feature or request backlog labels Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants