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

Add return value to void node_handle.param #753

Closed
NikolasE opened this issue Feb 23, 2016 · 5 comments
Closed

Add return value to void node_handle.param #753

NikolasE opened this issue Feb 23, 2016 · 5 comments

Comments

@NikolasE
Copy link

There is no way to see if the default value for a loaded parameter was used or if it was read from the parameter server. The function currently does not return anything so it would be simple to improve it like this:

  template<typename T>
  **bool** param(const std::string& param_name, T& param_val, const T& default_val) const
  {
    if (hasParam(param_name))
    {
      if (getParam(param_name, param_val))
      {
        return **true**;
      }
    }
    param_val = default_val;
    return **false**;
}

What do you think? Current code would just ignore the return value so that it should not create a problem for anyone.

@dirk-thomas
Copy link
Member

The proposed change would change ABI and therefore not be consider for already released distros (Indigo, Jade). But it could easily be implemented for Kinetic since it doesn't require any change in user code. Would you be interested to provide a pull request including some tests once there is a kinetic-devel branch?

There is a simple way to achieve the desired behavior with the existing API with the only downside that it requires more then one line. You can call bool getParam(key, & value) which will set the parameter value if it is available on the parameter server and if the return value is false you have to set the variable to a default value manually.

@NikolasE
Copy link
Author

NikolasE commented Mar 2, 2016

Hey! I'm going to write the pull request. Maybe we could also (sooner) add a
bool get_setter("my_param", param_value, default_value)
that would return hasParam("my_param") and would either read the value from the server or use the default_value and write it to the server.

@dirk-thomas
Copy link
Member

I am not sure that it is a good idea that a function caled get_* actually writes any data to the server. But it could certainly provide the two separate steps in a single function.

@dirk-thomas
Copy link
Member

Please see #775 for the proposed change in Kinetic.

@NikolasE
Copy link
Author

Look perfect. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants