From 578ee0b4d9d9a0e858b4a568e6a23be257367732 Mon Sep 17 00:00:00 2001 From: Owen-Liuyuxuan Date: Mon, 4 Dec 2023 16:31:35 +0800 Subject: [PATCH 1/7] only check arguments with the first inclusion Signed-off-by: Owen-Liuyuxuan --- .../actions/include_launch_description.py | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/launch/launch/actions/include_launch_description.py b/launch/launch/actions/include_launch_description.py index d022d5ee5..e27005661 100644 --- a/launch/launch/actions/include_launch_description.py +++ b/launch/launch/actions/include_launch_description.py @@ -160,21 +160,23 @@ def execute(self, context: LaunchContext) -> List[LaunchDescriptionEntity]: perform_substitutions(context, normalize_to_list_of_substitutions(arg_name)) for arg_name, arg_value in self.launch_arguments ] - declared_launch_arguments = ( - launch_description.get_launch_arguments_with_include_launch_description_actions()) - for argument, ild_actions in declared_launch_arguments: - if argument._conditionally_included or argument.default_value is not None: - continue - argument_names = my_argument_names - if ild_actions is not None: - for ild_action in ild_actions: - argument_names.extend(ild_action._try_get_arguments_names_without_context()) - if argument.name not in argument_names: - raise RuntimeError( - "Included launch description missing required argument '{}' " - "(description: '{}'), given: [{}]" - .format(argument.name, argument.description, ', '.join(argument_names)) - ) + if 'flag_argument_checked' not in context.get_locals_as_dict(): + declared_launch_arguments = ( + launch_description.get_launch_arguments_with_include_launch_description_actions()) + for argument, ild_actions in declared_launch_arguments: + if argument._conditionally_included or argument.default_value is not None: + continue + argument_names = my_argument_names + if ild_actions is not None: + for ild_action in ild_actions: + argument_names.extend(ild_action._try_get_arguments_names_without_context()) + if argument.name not in argument_names: + raise RuntimeError( + "Included launch description missing required argument '{}' " + "(description: '{}'), given: [{}]" + .format(argument.name, argument.description, ', '.join(argument_names)) + ) + context.extend_locals({'flag_argument_checked': True}) # Create actions to set the launch arguments into the launch configurations. set_launch_configuration_actions = [] From 8963283f5de2e8972d4157eb72ab77407424a41a Mon Sep 17 00:00:00 2001 From: Owen-Liuyuxuan Date: Mon, 4 Dec 2023 17:19:31 +0800 Subject: [PATCH 2/7] fix line too long CI Signed-off-by: Owen-Liuyuxuan --- launch/launch/actions/include_launch_description.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/launch/launch/actions/include_launch_description.py b/launch/launch/actions/include_launch_description.py index e27005661..474975060 100644 --- a/launch/launch/actions/include_launch_description.py +++ b/launch/launch/actions/include_launch_description.py @@ -169,7 +169,9 @@ def execute(self, context: LaunchContext) -> List[LaunchDescriptionEntity]: argument_names = my_argument_names if ild_actions is not None: for ild_action in ild_actions: - argument_names.extend(ild_action._try_get_arguments_names_without_context()) + argument_names.extend( + ild_action._try_get_arguments_names_without_context() + ) if argument.name not in argument_names: raise RuntimeError( "Included launch description missing required argument '{}' " From c4c359cd353e0c3149c833fdc10142b8bca5f2c4 Mon Sep 17 00:00:00 2001 From: Owen-Liuyuxuan Date: Tue, 12 Dec 2023 13:13:30 +0800 Subject: [PATCH 3/7] skip include checking Signed-off-by: Owen-Liuyuxuan --- .../actions/include_launch_description.py | 34 +++++++++---------- launch/launch/launch_description.py | 2 ++ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/launch/launch/actions/include_launch_description.py b/launch/launch/actions/include_launch_description.py index 474975060..67e37b857 100644 --- a/launch/launch/actions/include_launch_description.py +++ b/launch/launch/actions/include_launch_description.py @@ -160,25 +160,23 @@ def execute(self, context: LaunchContext) -> List[LaunchDescriptionEntity]: perform_substitutions(context, normalize_to_list_of_substitutions(arg_name)) for arg_name, arg_value in self.launch_arguments ] - if 'flag_argument_checked' not in context.get_locals_as_dict(): - declared_launch_arguments = ( - launch_description.get_launch_arguments_with_include_launch_description_actions()) - for argument, ild_actions in declared_launch_arguments: - if argument._conditionally_included or argument.default_value is not None: - continue - argument_names = my_argument_names - if ild_actions is not None: - for ild_action in ild_actions: - argument_names.extend( - ild_action._try_get_arguments_names_without_context() - ) - if argument.name not in argument_names: - raise RuntimeError( - "Included launch description missing required argument '{}' " - "(description: '{}'), given: [{}]" - .format(argument.name, argument.description, ', '.join(argument_names)) + declared_launch_arguments = ( + launch_description.get_launch_arguments_with_include_launch_description_actions()) + for argument, ild_actions in declared_launch_arguments: + if argument._conditionally_included or argument.default_value is not None: + continue + argument_names = my_argument_names + if ild_actions is not None: + for ild_action in ild_actions: + argument_names.extend( + ild_action._try_get_arguments_names_without_context() ) - context.extend_locals({'flag_argument_checked': True}) + if argument.name not in argument_names: + raise RuntimeError( + "Included launch description missing required argument '{}' " + "(description: '{}'), given: [{}]" + .format(argument.name, argument.description, ', '.join(argument_names)) + ) # Create actions to set the launch arguments into the launch configurations. set_launch_configuration_actions = [] diff --git a/launch/launch/launch_description.py b/launch/launch/launch_description.py index ac67de064..ba05da955 100644 --- a/launch/launch/launch_description.py +++ b/launch/launch/launch_description.py @@ -139,6 +139,8 @@ def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=Non entity._conditionally_included = _conditional_inclusion entity._conditionally_included |= entity.condition is not None declared_launch_arguments.append((entity, nested_ild_actions)) + if isinstance(entity, IncludeLaunchDescription): + continue if isinstance(entity, ResetLaunchConfigurations): # Launch arguments after this cannot be set directly by top level arguments return From 825596efdbc35a115d0fd377056bc1c058c89775 Mon Sep 17 00:00:00 2001 From: Owen-Liuyuxuan Date: Tue, 12 Dec 2023 13:29:04 +0800 Subject: [PATCH 4/7] only search local argument Signed-off-by: Owen-Liuyuxuan --- launch/launch/actions/include_launch_description.py | 2 +- launch/launch/launch_description.py | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/launch/launch/actions/include_launch_description.py b/launch/launch/actions/include_launch_description.py index 67e37b857..0aff960c4 100644 --- a/launch/launch/actions/include_launch_description.py +++ b/launch/launch/actions/include_launch_description.py @@ -161,7 +161,7 @@ def execute(self, context: LaunchContext) -> List[LaunchDescriptionEntity]: for arg_name, arg_value in self.launch_arguments ] declared_launch_arguments = ( - launch_description.get_launch_arguments_with_include_launch_description_actions()) + launch_description.get_launch_arguments_with_include_launch_description_actions(only_search_local=True)) for argument, ild_actions in declared_launch_arguments: if argument._conditionally_included or argument.default_value is not None: continue diff --git a/launch/launch/launch_description.py b/launch/launch/launch_description.py index ba05da955..f1d8514f6 100644 --- a/launch/launch/launch_description.py +++ b/launch/launch/launch_description.py @@ -91,7 +91,7 @@ def get_launch_arguments(self, conditional_inclusion=False) -> List[DeclareLaunc ] def get_launch_arguments_with_include_launch_description_actions( - self, conditional_inclusion=False + self, conditional_inclusion=False, only_search_local=False ) -> List[Tuple[DeclareLaunchArgument, List['IncludeLaunchDescription']]]: """ Return a list of launch arguments with its associated include launch descriptions actions. @@ -128,7 +128,7 @@ def get_launch_arguments_with_include_launch_description_actions( Tuple[DeclareLaunchArgument, List[IncludeLaunchDescription]]] = [] from .actions import ResetLaunchConfigurations - def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=None): + def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=None, only_search_local=False): for entity in entities: if isinstance(entity, DeclareLaunchArgument): # Avoid duplicate entries with the same name. @@ -139,8 +139,9 @@ def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=Non entity._conditionally_included = _conditional_inclusion entity._conditionally_included |= entity.condition is not None declared_launch_arguments.append((entity, nested_ild_actions)) - if isinstance(entity, IncludeLaunchDescription): - continue + if only_search_local: + if isinstance(entity, IncludeLaunchDescription): + continue if isinstance(entity, ResetLaunchConfigurations): # Launch arguments after this cannot be set directly by top level arguments return @@ -160,7 +161,7 @@ def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=Non _conditional_inclusion=True, nested_ild_actions=next_nested_ild_actions) - process_entities(self.entities, _conditional_inclusion=conditional_inclusion) + process_entities(self.entities, _conditional_inclusion=conditional_inclusion, only_search_local=only_search_local) return declared_launch_arguments From bd8b70a122039bd5fdefda72553bf4ee353c40ba Mon Sep 17 00:00:00 2001 From: Owen-Liuyuxuan Date: Tue, 12 Dec 2023 13:36:00 +0800 Subject: [PATCH 5/7] fix CI Signed-off-by: Owen-Liuyuxuan --- launch/launch/actions/include_launch_description.py | 4 +++- launch/launch/launch_description.py | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/launch/launch/actions/include_launch_description.py b/launch/launch/actions/include_launch_description.py index 0aff960c4..184abf4d1 100644 --- a/launch/launch/actions/include_launch_description.py +++ b/launch/launch/actions/include_launch_description.py @@ -161,7 +161,9 @@ def execute(self, context: LaunchContext) -> List[LaunchDescriptionEntity]: for arg_name, arg_value in self.launch_arguments ] declared_launch_arguments = ( - launch_description.get_launch_arguments_with_include_launch_description_actions(only_search_local=True)) + launch_description.get_launch_arguments_with_include_launch_description_actions( + only_search_local=True) + ) for argument, ild_actions in declared_launch_arguments: if argument._conditionally_included or argument.default_value is not None: continue diff --git a/launch/launch/launch_description.py b/launch/launch/launch_description.py index f1d8514f6..06e13fd50 100644 --- a/launch/launch/launch_description.py +++ b/launch/launch/launch_description.py @@ -128,7 +128,8 @@ def get_launch_arguments_with_include_launch_description_actions( Tuple[DeclareLaunchArgument, List[IncludeLaunchDescription]]] = [] from .actions import ResetLaunchConfigurations - def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=None, only_search_local=False): + def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=None, + only_search_local=False): for entity in entities: if isinstance(entity, DeclareLaunchArgument): # Avoid duplicate entries with the same name. @@ -161,7 +162,8 @@ def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=Non _conditional_inclusion=True, nested_ild_actions=next_nested_ild_actions) - process_entities(self.entities, _conditional_inclusion=conditional_inclusion, only_search_local=only_search_local) + process_entities(self.entities, _conditional_inclusion=conditional_inclusion, + only_search_local=only_search_local) return declared_launch_arguments From aaecb95637677cd29563086d0c7afd8768692267 Mon Sep 17 00:00:00 2001 From: Owen-Liuyuxuan Date: Tue, 12 Dec 2023 13:41:06 +0800 Subject: [PATCH 6/7] fix indention CI Signed-off-by: Owen-Liuyuxuan --- launch/launch/launch_description.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/launch/launch/launch_description.py b/launch/launch/launch_description.py index 06e13fd50..deb847300 100644 --- a/launch/launch/launch_description.py +++ b/launch/launch/launch_description.py @@ -129,7 +129,7 @@ def get_launch_arguments_with_include_launch_description_actions( from .actions import ResetLaunchConfigurations def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=None, - only_search_local=False): + only_search_local=False): for entity in entities: if isinstance(entity, DeclareLaunchArgument): # Avoid duplicate entries with the same name. @@ -163,7 +163,7 @@ def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=Non nested_ild_actions=next_nested_ild_actions) process_entities(self.entities, _conditional_inclusion=conditional_inclusion, - only_search_local=only_search_local) + only_search_local=only_search_local) return declared_launch_arguments From 767504cf2ea82a757c29afb5be1417bbbb430804 Mon Sep 17 00:00:00 2001 From: Owen-Liuyuxuan Date: Tue, 12 Dec 2023 14:02:46 +0800 Subject: [PATCH 7/7] fix bugs in looping inside Signed-off-by: Owen-Liuyuxuan --- launch/launch/launch_description.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/launch/launch/launch_description.py b/launch/launch/launch_description.py index deb847300..bc7d30379 100644 --- a/launch/launch/launch_description.py +++ b/launch/launch/launch_description.py @@ -155,12 +155,14 @@ def process_entities(entities, *, _conditional_inclusion, nested_ild_actions=Non process_entities( entity.describe_sub_entities(), _conditional_inclusion=False, - nested_ild_actions=next_nested_ild_actions) + nested_ild_actions=next_nested_ild_actions, + only_search_local=only_search_local) for conditional_sub_entity in entity.describe_conditional_sub_entities(): process_entities( conditional_sub_entity[1], _conditional_inclusion=True, - nested_ild_actions=next_nested_ild_actions) + nested_ild_actions=next_nested_ild_actions, + only_search_local=only_search_local) process_entities(self.entities, _conditional_inclusion=conditional_inclusion, only_search_local=only_search_local)