Add frontend parsing methods for Node, ExecutableInPackage and FindPackage substitution #23

ivanpauno · 2019-05-10T19:55:23Z

Add parsing method for Node and ExecutableInPackage.
Add test example of launching a node from xml.

Blocked by ros2/launch#226
Blocked by #33

hidmic

This looks amazing, great job @ivanpauno !

launch_ros/launch_ros/actions/node.py

launch_ros/launch_ros/substitutions/executable_in_package.py

test_launch_ros/test/test_launch_ros/xml_frontend/node.xml

test_launch_ros/test/test_launch_ros/xml_frontend/test_node_xml.py

ivanpauno · 2019-05-31T20:53:41Z

Rebased again with master

ivanpauno · 2019-06-03T14:28:26Z

@hidmic 218d879 and ros2/launch@cd4ff85 handles parameters correctly after #31.
If you're ok with that change, this is ready for be reviewed by someone else.

ivanpauno · 2019-06-03T21:07:12Z

@hidmic 218d879 and ros2/launch@cd4ff85 handles parameters correctly after #31.
If you're ok with that change, this is ready for be reviewed by someone else.

Reverted both commits after discussing with @hidmic.
See 86bd241 for checking how it looks now.

wjwwood

This also lgtm!

launch_ros/launch_ros/actions/node.py

test_launch_ros/test/test_launch_ros/launch_frontend/test_node_frontend.py

launch_ros/launch_ros/actions/node.py

launch_ros/launch_ros/substitutions/executable_in_package.py

hidmic · 2019-07-03T15:04:22Z

test_launch_ros/test/test_launch_ros/launch_frontend/test_node_frontend.py

+import pytest
+
+# Scaping the quote is needed in 'a_string' launch configuration, becuase of how the parser works.
+# TODO(ivanpauno): Check if it's possible to avoid that.


@ivanpauno is all this quoting for 'a_string' due to substitution grammar parsing? Is that bad for YAML markup?

Yeah, the substitution grammar needs the quotes escaped. In YAML markup you have the same problem, but it's not particularly "bad" for it.
The yaml example is in the same file (below the xml example).

test_launch_ros/test/test_launch_ros/launch_frontend/test_node_frontend.py

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

…ckage.xml. Move test descriptions in place Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

This reverts commit 218d879. Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

…ated test. Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Signed-off-by: ivan <ivanpauno@gmail.com>

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

ivanpauno · 2019-07-08T18:03:16Z

@hidmic I rebased, because I needed #33.
The new commits since your last review are: a3ca07f, 07f162d, c1c1d6a, 3a95e38.

wjwwood

New changes lgtm.

hidmic

LGTM too!

hidmic · 2019-07-10T19:44:14Z

launch_ros/launch_ros/actions/node.py

+        package = parser.parse_substitution(entity.get_attr('package'))
+        kwargs['package'] = package
+        executable = parser.parse_substitution(entity.get_attr('executable'))
+        kwargs['node_executable'] = executable


@ivanpauno just curious, why the two step assignment?

Just a bad habit.

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

hidmic

LGTM!

ivanpauno requested a review from hidmic May 10, 2019 19:55

ivanpauno self-assigned this May 10, 2019

ivanpauno added in progress in review and removed in progress labels May 10, 2019

ivanpauno mentioned this pull request May 10, 2019

Add frontend module in launch, launch_xml and launch_yaml packages ros2/launch#226

Merged

ivanpauno changed the title ~~Ivanpauno/launch frontend~~ Add launch_frontend parsing methods for Node and ExecutableInPackage May 10, 2019

hidmic requested changes May 15, 2019

View reviewed changes

ivanpauno force-pushed the ivanpauno/launch-frontend branch from cda8719 to c56dff8 Compare May 17, 2019 19:15

ivanpauno force-pushed the ivanpauno/launch-frontend branch from 3abfbe9 to 1282980 Compare May 31, 2019 20:52

ivanpauno force-pushed the ivanpauno/launch-frontend branch from f0e244c to 86bd241 Compare June 3, 2019 20:49

wjwwood approved these changes Jun 27, 2019

View reviewed changes

launch_ros/launch_ros/actions/node.py Outdated Show resolved Hide resolved

test_launch_ros/test/test_launch_ros/launch_frontend/test_node_frontend.py Outdated Show resolved Hide resolved

ivanpauno requested a review from hidmic July 2, 2019 20:08

hidmic reviewed Jul 3, 2019

View reviewed changes

ivanpauno added 13 commits July 8, 2019 14:51

Add node parsing method and test

8198671

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Used parse_substitution in parse Node method

3fbd877

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Corrected node parsing method and test with lastest changes

b93e86d

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Expose ExecutableInPackage substitution

a0de256

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Updated node parsing method with typed get_attr change.

9869bbd

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Correct param and env parsing in node parsing method

011f188

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Add yaml node example. Reoredered test folders

1d1559b

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Address review comments

7ff4c2f

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Allow code reusage with base class parsing method

6c986fa

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Parameterize node frontend tests. Add missing test dependencies in pa…

f5f88c5

…ckage.xml. Move test descriptions in place Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Handle parameters correctly after bb82f31

55df869

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Revert "Handle parameters correctly after bb82f31"

02184a3

This reverts commit 218d879. Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Deleted unused node.xml file

6799cdd

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

ivanpauno and others added 17 commits July 8, 2019 14:54

Better testing for node parsing

e2e8f7a

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Updated after addressing comments in launch PR

cd8a01f

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Address PR comments

df0b0dd

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Updated after merging launch_frontend into launch

3f67583

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Update after change of get_attr types argument in launch_frontend

f58b863

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Updated old import in test

0608922

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Correct param from option parsing. Add find-package substitution. Upd…

0a16d4e

…ated test. Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Avoid unnecessary installation of testing file

110fb1a

Signed-off-by: ivan <ivanpauno@gmail.com>

Address PR comments

b8b6b14

Signed-off-by: ivan <ivanpauno@gmail.com>

Renamed launch_frontend to launch

009041a

Signed-off-by: ivan <ivanpauno@gmail.com>

Correct Node parsing method after change in ExecuteProcess

dfc2292

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Add more test cases for loading params

7dede1f

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Allow parameter name to be a substitution

f202a63

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Rename launch_frontend test folder to frontend

a3ca07f

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Add new tests for node parameters

07f162d

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Update get_attr argument from types to data_type

c1c1d6a

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

Correct parameters parsing and test

3a95e38

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

ivanpauno force-pushed the ivanpauno/launch-frontend branch from f456720 to 3a95e38 Compare July 8, 2019 18:00

wjwwood approved these changes Jul 8, 2019

View reviewed changes

hidmic approved these changes Jul 10, 2019

View reviewed changes

Change code style. Delete old comment

840b279

Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>

ivanpauno changed the title ~~Add launch_frontend parsing methods for Node and ExecutableInPackage~~ Add frontend parsing methods for Node, ExecutableInPackage. Add FindPackage substitution Jul 11, 2019

ivanpauno changed the title ~~Add frontend parsing methods for Node, ExecutableInPackage. Add FindPackage substitution~~ Add frontend parsing methods for Node and ExecutableInPackage. Add FindPackage substitution Jul 11, 2019

ivanpauno changed the title ~~Add frontend parsing methods for Node and ExecutableInPackage. Add FindPackage substitution~~ Add frontend parsing methods for Node, ExecutableInPackage and FindPackage substitution Jul 11, 2019

hidmic approved these changes Jul 11, 2019

View reviewed changes

ivanpauno merged commit 5fac112 into master Jul 11, 2019

delete-merged-branch bot deleted the ivanpauno/launch-frontend branch July 11, 2019 16:25

wjwwood mentioned this pull request Jul 23, 2019

ROS 2 Eloquent Elusor ros2/ros2#734

Closed

34 tasks

ivanpauno mentioned this pull request Jul 30, 2019

XML/YAML Front-end ros2/launch#163

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add frontend parsing methods for Node, ExecutableInPackage and FindPackage substitution #23

Add frontend parsing methods for Node, ExecutableInPackage and FindPackage substitution #23

ivanpauno commented May 10, 2019 •

edited

hidmic left a comment

ivanpauno commented May 31, 2019

ivanpauno commented Jun 3, 2019

ivanpauno commented Jun 3, 2019

wjwwood left a comment

hidmic Jul 3, 2019

ivanpauno Jul 3, 2019

ivanpauno commented Jul 8, 2019

wjwwood left a comment

hidmic left a comment

hidmic Jul 10, 2019

ivanpauno Jul 11, 2019

hidmic left a comment

Add frontend parsing methods for Node, ExecutableInPackage and FindPackage substitution #23

Add frontend parsing methods for Node, ExecutableInPackage and FindPackage substitution #23

Conversation

ivanpauno commented May 10, 2019 • edited

hidmic left a comment

Choose a reason for hiding this comment

ivanpauno commented May 31, 2019

ivanpauno commented Jun 3, 2019

ivanpauno commented Jun 3, 2019

wjwwood left a comment

Choose a reason for hiding this comment

hidmic Jul 3, 2019

Choose a reason for hiding this comment

ivanpauno Jul 3, 2019

Choose a reason for hiding this comment

ivanpauno commented Jul 8, 2019

wjwwood left a comment

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

hidmic Jul 10, 2019

Choose a reason for hiding this comment

ivanpauno Jul 11, 2019

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

ivanpauno commented May 10, 2019 •

edited