Skip to content

Commit

Permalink
[bug] fixes #1158 (#1159)
Browse files Browse the repository at this point in the history
* [bug] fixes #1158

XmlLoader and LoaderContext no longer share the param list to child 'node' contexts.
This was causing the creation of unintended private parameters when the tilde notation was used.

* added test cases for tilde param in launch

* added test cases for tilde param in python

* fixed tilde param issue for group tags

Issue #1158 manifested for group tags that appear before (but not containing) node tags.

* added one more test case for issue #1158
used param tag to make sure we test the proposed fix

* Added negative tests for extra parameters

Some parameters should not be present at all.
  • Loading branch information
git-afsantos authored and dirk-thomas committed Oct 26, 2017
1 parent a915a99 commit b8fecba
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 0 deletions.
2 changes: 2 additions & 0 deletions tools/roslaunch/src/roslaunch/xmlloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ def _node_tag(self, tag, context, ros_config, default_machine, is_test=False, ve

child_ns = self._ns_clear_params_attr('node', tag, context, ros_config, node_name=name)
param_ns = child_ns.child(name)
param_ns.params = [] # This is necessary because child() does not make a copy of the param list.

# required attributes
pkg, node_type = self.reqd_attrs(tag, context, ('pkg', 'type'))
Expand Down Expand Up @@ -647,6 +648,7 @@ def _recurse_load(self, ros_config, tags, context, default_machine, is_core, ver
if ifunless_test(self, tag, context):
self._check_attrs(tag, context, ros_config, XmlLoader.GROUP_ATTRS)
child_ns = self._ns_clear_params_attr(name, tag, context, ros_config)
child_ns.params = list(child_ns.params) # copy is needed here to enclose new params
default_machine = \
self._recurse_load(ros_config, tag.childNodes, child_ns, \
default_machine, is_core, verbose)
Expand Down
7 changes: 7 additions & 0 deletions tools/roslaunch/test/unit/test_roslaunch_dump_params.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ def test_roslaunch(self):
'/node_rosparam/dict1/shoulders': 2,
'/node_rosparam/dict1/knees': 3,
'/node_rosparam/dict1/toes': 4,
'/node_rosparam/tilde1': 'foo',
'/node_rosparam/local_param': 'baz',

'/node_rosparam2/tilde1': 'foo',

'/inline_str': 'value1',
'/inline_list': [1, 2, 3, 4],
Expand All @@ -99,3 +103,6 @@ def test_roslaunch(self):
elif v != output_val[k]:
self.fail("key [%s] value [%s] does not match output: %s"%(k, v, output_val[k]))
self.assertEquals(val, output_val)
for k in ('/node_rosparam/tilde2', '/node_rosparam2/tilde2', '/node_rosparam2/local_param'):
if k in output_val:
self.fail("key [%s] should not be in output: %s"%(k, output_val))
6 changes: 6 additions & 0 deletions tools/roslaunch/test/xml/test-dump-rosparam.launch
Original file line number Diff line number Diff line change
@@ -1,15 +1,21 @@
<launch>

<param name="~tilde1" value="foo" />
<rosparam file="$(find roslaunch)/test/dump-params.yaml" command="load" />

<group ns="rosparam">
<param name="~tilde2" value="bar" />
<rosparam file="$(find roslaunch)/test/dump-params.yaml" command="load" />
</group>

<node pkg="package" type="test_base" name="node_rosparam">
<param name="local_param" value="baz" />
<rosparam file="$(find roslaunch)/test/dump-params.yaml" command="load" />
</node>

<node pkg="package" type="test_base" name="node_rosparam2">
</node>

<rosparam param="inline_str">value1</rosparam>
<rosparam param="inline_list">[1, 2, 3, 4]</rosparam>
<rosparam param="inline_dict">{key1: value1, key2: value2}</rosparam>
Expand Down

0 comments on commit b8fecba

Please sign in to comment.