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

Make C++ Config instances have default values in constructor #107

Open
wants to merge 1 commit into
base: noetic-devel
Choose a base branch
from

Conversation

lucasw
Copy link

@lucasw lucasw commented May 18, 2018

This generates a header with an initializer list with all the default values, previously the defaults were loaded in only when the server ran.

This works now:

...
gen.add("str_param", str_t, 1, "string", "test")
...
#include <dynamic_reconfigure_example/ExampleConfig.h>
...
dynamic_reconfigure_example::ExampleConfig config;
ROS_INFO_STREAM(config.str_param);

For #33

(There isn't a corresponding class created for python (would that be useful?), the defaults can be accessed currently like this:

from dynamic_reconfigure_example.cfg import ExampleConfig
print ExampleConfig.defaults['str_param']

)

@mikaelarguedas
Copy link
Member

Sorry @lucasw for the wait and thanks for the contribution!

The line number comments came in by default and I would rather not have them, how to get rid of them?

There doesn't seem to be an no option to disable them in dynamic_reconfigure ATM.

A unit test would be good but haven't looked into that.

👍 A unit test for this would be great

Haven't looked at why there is redundant code in parameter_generator_catkin.py and parameter_generator.py

parameter_generator.py was for rosbuild support and is now deprecated, it will be removed in a future ROS Distribution. I would recommend not modifying it in this PR and keep scope the changes to parameter_generator_catkin.py

There isn't a corresponding class created for python

Yeah it would make sense to have the symmetric in Python, though it shouldn't block the C++ version from moving forward as there is an easy way to get them


@ros-pull-request-builder retest this please

@mikaelarguedas mikaelarguedas changed the base branch from master to melodic-devel October 2, 2018 19:01
@rowandempster
Copy link

What's the status on this?

@lucasw lucasw force-pushed the init_default_values branch from 941fb7d to 2795b6d Compare October 24, 2021 23:14
@lucasw lucasw changed the base branch from melodic-devel to noetic-devel October 24, 2021 23:15
@lucasw
Copy link
Author

lucasw commented Oct 25, 2021

Rebased this on noetic-devel, which causes the melodic build check to fail?

@lucasw lucasw force-pushed the init_default_values branch from 2795b6d to 6c39635 Compare May 8, 2022 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants