ros / dynamic_reconfigure

BSD 3-Clause "New" or "Revised" License
48 stars 111 forks source link

C++ dynamic reconfigure server with str_t param ignores numerical strings #101

Closed lucasw closed 6 years ago

lucasw commented 6 years ago

https://github.com/lucasw/dynamic_reconfigure_tools/tree/master/dynamic_reconfigure_example is an example C++ server with a str_t param that it prints out when reconfigured:

...
gen.add("str_param", str_t, 1, "string", "")
...

Alphanumeric strings work as expected:

$ rosrun dynamic_reconfigure_example example_server_node _str_param:=5a
[ INFO] [/example_server] :[233] [Initializing nodelet with 8 worker threads.]
[ INFO] [/example_server] :[56] [5a]

But an int or float string gets dropped entirely in favor of the default "" value:

$ rosrun dynamic_reconfigure_example example_server_node _str_param:=55
[ INFO] [/example_server] [233] [Initializing nodelet with 8 worker threads.]
[ INFO] [/example_server] [56] []

The server also happens to be a nodelet, I don't think that is related but I'll create a non-nodelet example server later.

The same thing works fine with a python server (the server prints out the entire config on reconfigure):

$ rosrun rqt_dr_single example_server.py _str_param:="12345"
[INFO] [/rqt_dr_single_13663_1520016082376] :[15] [Reconfigure Request: 50, 0.5,\ 
          12345, True, 1]
lucasw commented 6 years ago

It looks like this is a problem with C++ getParam and nothing to do with dynamic reconfigure.

https://github.com/ros/ros_comm/issues/1339

lucasw commented 6 years ago

My current workaround is to use an int type for the dynamic reconfigure value and converting it back to a string inside the reconfigure callback, though this risks a inexact string match in the case of leading zeros (fortunately I'm not dealing with floats).

(type="str" is fine for roslaunch params, but having to remember different conventions depending on command line vs. roslaunch is error prone)

mikaelarguedas commented 6 years ago

Glad you found a workaround.

Thanks for providing the link to the upstream issue :+1: , that'll be helpful for people running into the some issue and landing on this ticket.