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

roscpp missing getParam() for lists and dictionaries #263

Closed
b-adkins opened this issue Aug 2, 2013 · 5 comments
Closed

roscpp missing getParam() for lists and dictionaries #263

b-adkins opened this issue Aug 2, 2013 · 5 comments

Comments

@b-adkins
Copy link

b-adkins commented Aug 2, 2013

"roscpp's parameter API supports all of [ROS Parameter Server's data types], though it is only easy to use strings, integers, floats and booleans." (roscpp Parameter Server)

Why? I don't see why an application developer needs to learn XmlRpc and re-implement the wheel. I believe it's quite doable to reconstruct lists and dictionaries (at least of primitives) entered in YAML to the parameter server.

I propose a patch with templated varieties of NodeHandle::getParam() and param::get() that:

Extracts lists as

// C++ STL style:
std::vector<type> the_list_cpp;

// Possibly C style:
type* the_list_c;

Extracts dictionaries as

// C++ STL style:
std::map<std::string, type> the_dictionary;

// C style:
char* keys[];
type* values;

Proposed interface:
(n_max is the maximum elements the passed buffer(s) can hold, to prevent buffer overrun)

// C++ STL style:
template<typename T> bool getParam(const std:string& key, std::vector<T>& l) const;
template<typename T> bool getParam(const std:string& key, std::map<std::string, T>& d) const;

// C style:
template<typename T> bool getParam(const std:string& key, T* l, int n_max) const;
template<typename T> bool getParam(const std:string& key, char* s[], T* v, int n_max) const;

The C++-style versions would be very easy to use. The C-style types may be useful for performance.

I've essentially already written these for another project. I would like to offer my code as part of this proposed patch.

EDIT: For easy reference, here is <ros/param.h>:
https://github.com/ros/ros_comm/blob/groovy-devel/clients/roscpp/include/ros/param.h

Also note that the methods would need to be wrapped NodeHandle:
https://github.com/ros/ros_comm/blob/groovy-devel/clients/roscpp/src/libros/node_handle.cpp

@jbohren
Copy link
Member

jbohren commented Aug 21, 2013

I think many of us have implemented similar things. It would be great to see some of them added to the core libraries.
See my vector param implementation here: https://github.com/jhu-lcsr/terse_roscpp/blob/master/include/terse_roscpp/param.h

Do you have an implementation of your own floating around somewhere?

@b-adkins
Copy link
Author

Sorry for the delayed response. I have getters for vectors and maps
which use the same conventions as the ros code. I'll post them when I
figure out how to use GitHub.

Bea

On 8/21/2013 8:54 AM, Jonathan Bohren wrote:

I think many of us have implemented similar things. It would be great
to see some of them added to the core libraries.
See my vector param implementation here:
https://github.com/jhu-lcsr/terse_roscpp/blob/master/include/terse_roscpp/param.h

Do you have an implementation of your own floating around somewhere?


Reply to this email directly or view it on GitHub
#263 (comment).

@b-adkins
Copy link
Author

Here is my code.
https://gist.github.com/b-adkins/6362327

Some thoughts:

  • I made the style copy-pasteable right into <ros/param.h>.
  • The templates can fill a vector or a map with a single type of value, thus the underlying param server value must convertible to that type.
  • On the variable names, I chose "d" for dictionary and "l" for list (as they are called in YAML).
  • By creating an isType(testee) method, it has shorter and less repetitive template specialization.

@jbohren
Copy link
Member

jbohren commented Sep 3, 2013

@b-adkins The best way to contribute a patch is by "forking" a given repository, making your changes in the fork, and then creating a "pull request" (tutorial here). I've been waiting for three years or so for someone to implement these features, and I think part of it is because while most submissions work, they're not up to the standards of the ROS dev team. I took some time thisafternoon to create a patch with a similar scope, but a little more polished: #279 and hopefully this can get merged in soon and roscpp can stop envying rospy...

@dirk-thomas
Copy link
Member

PR has been merged and package released.

contradict pushed a commit to contradict/ros_comm that referenced this issue Aug 12, 2016
White background to contrast with black pens
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

3 participants