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.

Sign up for GitHub

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

[roslaunch] Add yaml type for <param> tag #1045

Merged
merged 11 commits into from
Jul 7, 2017
Merged

[roslaunch] Add yaml type for <param> tag #1045

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b44af6c
Add yaml type for <param> tag
wkentaro May 2, 2017
242cfcd
Support type for command output of <param> tag
wkentaro May 2, 2017
7c80460
Add test for yaml type
wkentaro May 2, 2017
0112b87
Add test for identifying commandline output as types
wkentaro May 2, 2017
ea65411
Support yaml type with textfile for <param>
wkentaro May 3, 2017
b746f71
Reserve current behavior with handling strip cleanly
wkentaro May 16, 2017
5b8718c
Revert <test_depend>rospy</test_depend>: no need
wkentaro May 22, 2017
2d8ae09
Revert no need strip()
wkentaro May 22, 2017
6e3e0b0
Remove the lazy import for yaml
wkentaro May 22, 2017
412d56e
Raise ValueError for yaml parse error
wkentaro May 25, 2017
c907f36
Strip for boolean rosparam
wkentaro May 25, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions tools/roslaunch/package.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
<run_depend version_gte="1.13.3">rosunit</run_depend>

<test_depend>rosbuild</test_depend>
<test_depend>rospy</test_depend>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary? I don't see any part in the patch which requires it.


<export>
<rosdoc config="rosdoc.yaml"/>
Expand Down
24 changes: 15 additions & 9 deletions tools/roslaunch/src/roslaunch/loader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,30 +70,36 @@ def convert_value(value, type_):
#attempt numeric conversion
try:
if '.' in value:
return float(value)
return float(value.strip())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary. float() already works even if the passed argument has leading / trailing spaces.

Same below.

else:
return int(value)
return int(value.strip())
except ValueError as e:
pass
#bool
lval = value.lower()
lval = value.strip().lower()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which case do you expect the strings true and false to have leading / trailing spaces?

if lval == 'true' or lval == 'false':
return convert_value(value, 'bool')
#string
return value
elif type_ == 'str' or type_ == 'string':
return value
elif type_ == 'int':
return int(value)
return int(value.strip())
elif type_ == 'double':
return float(value)
return float(value.strip())
elif type_ == 'bool' or type_ == 'boolean':
value = value.lower()
value = value.strip().lower()
if value == 'true' or value == '1':
return True
elif value == 'false' or value == '0':
return False
raise ValueError("%s is not a '%s' type"%(value, type_))
elif type_ == 'yaml':
# - lazy import
global yaml
if yaml is None:
import yaml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the import to the top of the file and avoid the conditional block.

return yaml.load(value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is that represented on the parameter server? Does this store a complex type (e.g. a nested dictionary) under a single name?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is set like this:

% rosparam get /test1
bin_contents:
  bin_I: [sharpie_accent_tank_style_highlighters, dr_browns_bottle_brush]
work_order:
- {bin: bin_I, item: sharpie_accent_tank_style_highlighters}

% rosparam get /test1/bin_contents/bin_I
[sharpie_accent_tank_style_highlighters, dr_browns_bottle_brush]


% rosparam get /test1/bin_contents
bin_I: [sharpie_accent_tank_style_highlighters, dr_browns_bottle_brush]

% rosparam get /test1/bin_contents/bin_I
[sharpie_accent_tank_style_highlighters, dr_browns_bottle_brush]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should any yaml specific exceptions be catched here and a ValueError be raised instead?

else:
raise ValueError("Unknown type '%s'"%type_)

Expand Down Expand Up @@ -465,10 +471,10 @@ def param_value(self, verbose, name, ptype, value, textfile, binfile, command):
@raise ValueError: if parameters are invalid
"""
if value is not None:
return convert_value(value.strip(), ptype)
return convert_value(value, ptype)
elif textfile is not None:
with open(textfile, 'r') as f:
return f.read()
return convert_value(f.read(), ptype)
elif binfile is not None:
try:
from xmlrpc.client import Binary
Expand Down Expand Up @@ -498,7 +504,7 @@ def param_value(self, verbose, name, ptype, value, textfile, binfile, command):
raise
if c_value is None:
raise ValueError("parameter: unable to get output of command [%s]"%command)
return c_value
return convert_value(c_value, ptype)
else: #_param_tag prevalidates, so this should not be reachable
raise ValueError("unable to determine parameter value")

9 changes: 9 additions & 0 deletions tools/roslaunch/test/xml/test-params-valid.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@

<param name="somefloat1" value="3.14159" type="double" />
<param name="somefloat2" value="5.0" />

<param name="someyaml1" value="[0, 1, 2]" type="yaml" />
<param name="someyaml2" value="{string1: bar2, string2: '10', integer1: 1, float1: 3.14, list1: [0, 1, 2], list2: ['a', 'b', 'c']}" type="yaml" />

<!-- you can set parameters in child namespaces -->
<param name="wg/wgchildparam" value="a child namespace parameter 1" />
Expand All @@ -32,6 +35,12 @@
<!-- upload the output of a command as a param. -->
<param name="commandoutput" command="cat &quot;$(find roslaunch)/resources/example.launch&quot;" />

<!-- upload the output of a command as a param as types. -->
<param name="commandoutputinteger" type="int" command="echo 0" />
<param name="commandoutputdouble" type="double" command="echo 10" />
<param name="commandoutputbool" type="bool" command="echo true" />
<param name="commandoutputyaml" type="yaml" command="cat &quot;$(find roslaunch)/test/params.yaml&quot;" />


<!-- test that we can override params -->
<param name="override" value="fail" />
Expand Down