-
Notifications
You must be signed in to change notification settings - Fork 27
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
Support loading lists of booleans from parameter server #6
base: melodic-devel
Are you sure you want to change the base?
Support loading lists of booleans from parameter server #6
Conversation
Would it make sense to use templating for |
Oh and target branch is |
Avoids having to create multiple variants for vectors with different value_types.
c41efd8
to
f7fdbdf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gavanderhoorn thanks for creating this PR! Would you mind adding the bool list param to the example implementation and README as well?
NodeHandle::getParam(..) supports a lot of overloads, many more than rosparam_shortcuts exposes through get(..).
Would it make sense to use templating for get(..) as well to be able to pass that through to NodeHandle::getParam(..) or would that not be of interest?
I think it definitely makes sense to use template functions where possible. It should be just a matter of fixing the debug strings for the different value types which could probably be done using template functions as well, just like your added getDebugArrayString() template.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great improvement. I like Henning's point about the README
@gavanderhoorn, when I switched to using the master branch of MoveIt (from melodic-devel), using the add_vector_of_bool branch broke compiling moveit for me with this error (see below). I fixed it by moving the template specialization to the .cpp file instead of the .h file (PR to our fork here) -- let me know if you'd like me to make a PR to your branch here as well.
|
Seems I forgot to add |
I'd love to merge this in, if we can update the README and address the inline issue |
Something I needed and might be of use to others.
This PR also changes
getDebugArrayString(..)
to a templated function, so as to avoid having to create versions for all the different types ofstd::vector<..>
thatNodeHandle::getParam(..)
supports.