From a1afeb7009e331140607ee6aa76f99291d597524 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Santos?= Date: Thu, 7 Sep 2017 13:35:46 +0100 Subject: [PATCH 1/6] [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. --- tools/roslaunch/src/roslaunch/xmlloader.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/roslaunch/src/roslaunch/xmlloader.py b/tools/roslaunch/src/roslaunch/xmlloader.py index 9136f09ee1..5e8a898ac7 100644 --- a/tools/roslaunch/src/roslaunch/xmlloader.py +++ b/tools/roslaunch/src/roslaunch/xmlloader.py @@ -370,6 +370,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')) From 05efac9a9b9e43f3068379987a00e1cdbefcb547 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Santos?= Date: Thu, 7 Sep 2017 14:40:55 +0100 Subject: [PATCH 2/6] added test cases for tilde param in launch --- tools/roslaunch/test/xml/test-dump-rosparam.launch | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/roslaunch/test/xml/test-dump-rosparam.launch b/tools/roslaunch/test/xml/test-dump-rosparam.launch index d02d34fa30..3ebfd0162f 100644 --- a/tools/roslaunch/test/xml/test-dump-rosparam.launch +++ b/tools/roslaunch/test/xml/test-dump-rosparam.launch @@ -1,8 +1,10 @@ + + @@ -10,6 +12,9 @@ + + + value1 [1, 2, 3, 4] {key1: value1, key2: value2} From 9bd65a65ffecc3c0cf661ccd89573a6e144ac1e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Santos?= Date: Thu, 7 Sep 2017 14:48:26 +0100 Subject: [PATCH 3/6] added test cases for tilde param in python --- tools/roslaunch/test/unit/test_roslaunch_dump_params.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/roslaunch/test/unit/test_roslaunch_dump_params.py b/tools/roslaunch/test/unit/test_roslaunch_dump_params.py index f1ba196174..2e8f277890 100644 --- a/tools/roslaunch/test/unit/test_roslaunch_dump_params.py +++ b/tools/roslaunch/test/unit/test_roslaunch_dump_params.py @@ -77,6 +77,9 @@ def test_roslaunch(self): '/node_rosparam/dict1/shoulders': 2, '/node_rosparam/dict1/knees': 3, '/node_rosparam/dict1/toes': 4, + '/node_rosparam/tilde1': 'foo', + + '/node_rosparam2/tilde1': 'foo', '/inline_str': 'value1', '/inline_list': [1, 2, 3, 4], From ac06491ff3a3a8be61f733673497b1caa741e742 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Santos?= Date: Thu, 7 Sep 2017 15:07:44 +0100 Subject: [PATCH 4/6] fixed tilde param issue for group tags Issue #1158 manifested for group tags that appear before (but not containing) node tags. --- tools/roslaunch/src/roslaunch/xmlloader.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/roslaunch/src/roslaunch/xmlloader.py b/tools/roslaunch/src/roslaunch/xmlloader.py index 5e8a898ac7..5feb0c4155 100644 --- a/tools/roslaunch/src/roslaunch/xmlloader.py +++ b/tools/roslaunch/src/roslaunch/xmlloader.py @@ -650,6 +650,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) From c71e1ac96a48fc2dedcee3752784b54873cde683 Mon Sep 17 00:00:00 2001 From: Andre Santos Date: Thu, 7 Sep 2017 15:16:53 +0100 Subject: [PATCH 5/6] added one more test case for issue #1158 used param tag to make sure we test the proposed fix --- tools/roslaunch/test/unit/test_roslaunch_dump_params.py | 1 + tools/roslaunch/test/xml/test-dump-rosparam.launch | 1 + 2 files changed, 2 insertions(+) diff --git a/tools/roslaunch/test/unit/test_roslaunch_dump_params.py b/tools/roslaunch/test/unit/test_roslaunch_dump_params.py index 2e8f277890..cc7b920640 100644 --- a/tools/roslaunch/test/unit/test_roslaunch_dump_params.py +++ b/tools/roslaunch/test/unit/test_roslaunch_dump_params.py @@ -78,6 +78,7 @@ def test_roslaunch(self): '/node_rosparam/dict1/knees': 3, '/node_rosparam/dict1/toes': 4, '/node_rosparam/tilde1': 'foo', + '/node_rosparam/local_param': 'baz', '/node_rosparam2/tilde1': 'foo', diff --git a/tools/roslaunch/test/xml/test-dump-rosparam.launch b/tools/roslaunch/test/xml/test-dump-rosparam.launch index 3ebfd0162f..cd8a6d9769 100644 --- a/tools/roslaunch/test/xml/test-dump-rosparam.launch +++ b/tools/roslaunch/test/xml/test-dump-rosparam.launch @@ -9,6 +9,7 @@ + From e479ba98df4e261fbf0193bf000e574b7b5d3cb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Santos?= Date: Fri, 8 Sep 2017 14:54:38 +0100 Subject: [PATCH 6/6] Added negative tests for extra parameters Some parameters should not be present at all. --- tools/roslaunch/test/unit/test_roslaunch_dump_params.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/roslaunch/test/unit/test_roslaunch_dump_params.py b/tools/roslaunch/test/unit/test_roslaunch_dump_params.py index cc7b920640..20024a6013 100644 --- a/tools/roslaunch/test/unit/test_roslaunch_dump_params.py +++ b/tools/roslaunch/test/unit/test_roslaunch_dump_params.py @@ -103,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))