Feature clear launch configs #515

djchopp · 2021-06-21T13:25:40Z

Adds the feature/functionality described in issue #507 such that launch configurations can be cleared within a current scope. The ClearLaunchConfigurations action compliments the Push* and Pop* actions with the following use pattern:

PushLaunchConfigurations()
ClearLaunchConfigurations()
# Scoped and isolated actions here
PopLaunchConfigurations()

Additionally the GroupAction has been updated to also support clearing of the launch configurations utilizing the pattern above.

launch/launch/actions/clear_launch_configurations.py

launch/launch/actions/group_action.py

wjwwood

Concept seems solid to me, as well as implementation. If we can clear up some documentation/interface concerns and add a few tests, I think it will be good to go.

launch/launch/actions/clear_launch_configurations.py

wjwwood

Documentation looks good to me, and implementation makes sense.

I did request one change related to the default argument, but I think it can trivially be fixed by using None.

I would still like to see some tests for this new action and extensions to the group action tests.

launch/launch/actions/clear_launch_configurations.py

wjwwood

One point for discussion, but looking good otherwise, maybe time to bring it out of draft?

launch/launch/actions/group_action.py

wjwwood

lgtm, several nitpicks but nothing serious

launch/launch/actions/group_action.py

launch/launch/actions/reset_launch_configurations.py

hidmic

Overall I like this patch. Left a few minor comments here and there, but otherwise this is pretty much ready to fly.

launch/launch/actions/reset_launch_configurations.py

launch/test/launch/actions/test_group_action.py

launch/test/launch/actions/test_reset_launch_configurations.py

hidmic

LGTM ! CI up to launch:

Linux
Linux-aarch64
macOS
Windows

djchopp · 2021-08-23T18:29:19Z

@hidmic Can you rerun the CI? Latest commit should fix the flake errors from the previous CI builds.

hidmic · 2021-08-23T18:52:19Z

Sure thing. Take two:

Linux
Linux-aarch64
macOS
Windows

hidmic · 2021-08-24T15:45:46Z

Alright, all green. We should probably run a more comprehensive CI now. @ivanpauno @wjwwood mind to take a look before I do that?

launch/launch/actions/group_action.py

ivanpauno

LGTM!
Thanks for the contribution @djchopp !

launch/launch/actions/reset_launch_configurations.py

hidmic

Overall this looks great to me @djchopp.

Thanks for the work and the patience.

launch_yaml/test/launch_yaml/test_reset.py

launch/launch/actions/group_action.py

hidmic

LGTM pending green CI:

Linux
Linux-aarch64
macOS
Windows

Thank you for patience @djchopp!

hidmic · 2021-09-29T17:12:20Z

@djchopp would you mind rebasing on top of master? That seems to be the cause behind CI failures.

djchopp · 2021-09-29T18:41:45Z

@hidmic This is nearing the edge of my git knowledge so hopefully I did that correctly. Let me know if I did something stupid.

hidmic · 2021-09-29T18:43:09Z

@djchopp did you merge master into your branch along the way at some point? I see commits that do not belong here.

djchopp · 2021-09-29T19:14:15Z

@hidimic Very possibly. Steps I did:

Fetch upstream in github
Pull origin master branch
Checkout local feat branch
rebase on master
Pull (merge) origin feat branch
Push local feat branch
Forget to signoff, rebase again with signoff
Work through merge conflicts
Push local feat branch, force with lease

If you know how to fix, feel free to do so

Signed-off-by: djchopp <[email protected]>

doc: fix docblock for newline style. Co-authored-by: William Woodall <[email protected]> Signed-off-by: djchopp <[email protected]>

doc: fix punctuation Co-authored-by: William Woodall <[email protected]> Signed-off-by: djchopp <[email protected]>

Signed-off-by: djchopp <[email protected]>

Co-authored-by: Michel Hidalgo <[email protected]> Signed-off-by: djchopp <[email protected]>

Signed-off-by: djchopp <[email protected]>

djchopp · 2021-09-30T13:27:54Z

@hidmic Thanks for the lesson! I had a copy saved off so was able to recover. Let me know if that looks better now.

hidmic · 2021-09-30T16:29:24Z

Thanks @djchopp !

Firing up CI again:

Linux
Linux-aarch64
macOS
Windows

launch/launch/actions/group_action.py

wjwwood · 2021-10-04T20:06:41Z

Does this close #507?

hidmic · 2021-10-04T20:17:12Z

Does this close #507?

Indeed :)

hidmic requested review from ivanpauno and wjwwood June 22, 2021 17:18

hidmic reviewed Jun 22, 2021

View reviewed changes

launch/launch/actions/clear_launch_configurations.py Outdated Show resolved Hide resolved

launch/launch/actions/clear_launch_configurations.py Outdated Show resolved Hide resolved

launch/launch/actions/group_action.py Outdated Show resolved Hide resolved

wjwwood reviewed Jun 22, 2021

View reviewed changes

launch/launch/actions/clear_launch_configurations.py Outdated Show resolved Hide resolved

launch/launch/actions/clear_launch_configurations.py Outdated Show resolved Hide resolved

launch/launch/actions/clear_launch_configurations.py Outdated Show resolved Hide resolved

wjwwood requested changes Jun 23, 2021

View reviewed changes

launch/launch/actions/clear_launch_configurations.py Outdated Show resolved Hide resolved

wjwwood reviewed Jun 23, 2021

View reviewed changes

launch/launch/actions/group_action.py Outdated Show resolved Hide resolved

hidmic reviewed Jun 24, 2021

View reviewed changes

launch/launch/actions/group_action.py Outdated Show resolved Hide resolved

djchopp marked this pull request as ready for review June 28, 2021 20:20

djchopp requested a review from wjwwood July 9, 2021 16:22

wjwwood reviewed Jul 12, 2021

View reviewed changes

wjwwood changed the title ~~Feat clear launch configs~~ Feature clear launch configs Jul 12, 2021

wjwwood added the enhancement New feature or request label Jul 12, 2021

clalancette assigned hidmic Aug 19, 2021

hidmic self-requested a review August 19, 2021 22:39

hidmic reviewed Aug 20, 2021

View reviewed changes

hidmic requested a review from wjwwood August 20, 2021 19:43

hidmic approved these changes Aug 20, 2021

View reviewed changes

ivanpauno reviewed Aug 25, 2021

View reviewed changes

launch/launch/actions/group_action.py Show resolved Hide resolved

ivanpauno approved these changes Aug 26, 2021

View reviewed changes

launch/launch/actions/reset_launch_configurations.py Show resolved Hide resolved

djchopp requested review from hidmic and ivanpauno September 16, 2021 17:10

hidmic reviewed Sep 16, 2021

View reviewed changes

launch_yaml/test/launch_yaml/test_reset.py Outdated Show resolved Hide resolved

launch/launch/actions/group_action.py Show resolved Hide resolved

hidmic approved these changes Sep 28, 2021

View reviewed changes

djchopp and others added 22 commits September 30, 2021 09:23

fix: default value of launch_configurations_to_not_be_cleared to None

fc8a809

Signed-off-by: djchopp <[email protected]>

add: Tests for new features

4430c6f

Signed-off-by: djchopp <[email protected]>

drop: ClearLaunchConfigurations

6bd1ba8

Signed-off-by: djchopp <[email protected]>

add: ResetLaunchConfigurations to replace ClearLaunchConfigurations

dd0135d

Signed-off-by: djchopp <[email protected]>

fix: update GroupAction to use new ResetLaunchConfigurations

5cfbe29

Signed-off-by: djchopp <[email protected]>

doc: update documentation to match ResetLaunchConfigurations usage

5da606a

Signed-off-by: djchopp <[email protected]>

Update launch/launch/actions/group_action.py

e8f1f85

doc: fix docblock for newline style. Co-authored-by: William Woodall <[email protected]> Signed-off-by: djchopp <[email protected]>

Update launch/launch/actions/reset_launch_configurations.py

b5c162c

doc: fix punctuation Co-authored-by: William Woodall <[email protected]> Signed-off-by: djchopp <[email protected]>

doc: update docblock to match style guide

226e60c

Signed-off-by: djchopp <[email protected]>

refactor: expand variable name abbreviations

0506fcf

Signed-off-by: djchopp <[email protected]>

doc: fix whitespace

1ad9f5e

Signed-off-by: djchopp <[email protected]>

doc: better wording

def3e9b

Co-authored-by: Michel Hidalgo <[email protected]> Signed-off-by: djchopp <[email protected]>

doc: better wording

9bb3450

Co-authored-by: Michel Hidalgo <[email protected]> Signed-off-by: djchopp <[email protected]>

refactor: use python dictionary update method

23954b5

Signed-off-by: djchopp <[email protected]>

add: verbose key and value checking

56f06be

Signed-off-by: djchopp <[email protected]>

refactor: corrected import order

2ad6141

Signed-off-by: djchopp <[email protected]>

add: parser supporting configuration reset

9a17b2c

Signed-off-by: djchopp <[email protected]>

fix: parse arg names as tuples (hashable) for dict key

2af8625

Signed-off-by: djchopp <[email protected]>

refactor: 'arg' to 'keep' tag

a184940

Signed-off-by: djchopp <[email protected]>

refactor: match dict creation method of other actions

ce7335a

Signed-off-by: djchopp <[email protected]>

add: xml, yaml tests for reset launch configuration functionality

cd96072

Signed-off-by: djchopp <[email protected]>

fix: use substitution instead of yaml node achor for tests

7417643

Signed-off-by: djchopp <[email protected]>

wjwwood approved these changes Oct 4, 2021

View reviewed changes

launch/launch/actions/group_action.py Show resolved Hide resolved

wjwwood merged commit ec481fb into ros2:master Oct 4, 2021

hidmic mentioned this pull request Oct 4, 2021

Ability to clear current launch configurations #507

Closed

clalancette mentioned this pull request Apr 29, 2022

ROS 2 Humble needed release notes ros2/ros2_documentation#2446

Closed

87 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature clear launch configs #515

Feature clear launch configs #515

djchopp commented Jun 21, 2021

wjwwood left a comment

wjwwood left a comment

wjwwood left a comment

wjwwood left a comment

hidmic left a comment

hidmic left a comment

djchopp commented Aug 23, 2021

hidmic commented Aug 23, 2021

hidmic commented Aug 24, 2021

ivanpauno left a comment

hidmic left a comment

hidmic left a comment •

edited

Loading

hidmic commented Sep 29, 2021

djchopp commented Sep 29, 2021

hidmic commented Sep 29, 2021

djchopp commented Sep 29, 2021

djchopp commented Sep 30, 2021

hidmic commented Sep 30, 2021

wjwwood commented Oct 4, 2021

hidmic commented Oct 4, 2021

Feature clear launch configs #515

Feature clear launch configs #515

Conversation

djchopp commented Jun 21, 2021

wjwwood left a comment

Choose a reason for hiding this comment

wjwwood left a comment

Choose a reason for hiding this comment

wjwwood left a comment

Choose a reason for hiding this comment

wjwwood left a comment

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

djchopp commented Aug 23, 2021

hidmic commented Aug 23, 2021

hidmic commented Aug 24, 2021

ivanpauno left a comment

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

hidmic left a comment • edited Loading

Choose a reason for hiding this comment

hidmic commented Sep 29, 2021

djchopp commented Sep 29, 2021

hidmic commented Sep 29, 2021

djchopp commented Sep 29, 2021

djchopp commented Sep 30, 2021

hidmic commented Sep 30, 2021

wjwwood commented Oct 4, 2021

hidmic commented Oct 4, 2021

hidmic left a comment •

edited

Loading