[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix no specified namespace #153

Merged
merged 5 commits into from
Jun 22, 2020
+57 −23
Merged

Fix no specified namespace #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c40c8ef
Mark namespace as not specified when it was not
ivanpauno Jun 19, 2020
3f94b0c
Correct test expectations
ivanpauno Jun 19, 2020
3933e35
Address peer review comments
ivanpauno Jun 19, 2020
6538d10
Address peer review comment
ivanpauno Jun 22, 2020
2e1f1ef
Extend node name tests
ivanpauno Jun 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 31 additions & 19 deletions launch_ros/launch_ros/actions/node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,18 @@
class Node(ExecuteProcess):
"""Action that executes a ROS node."""

UNSPECIFIED_NODE_NAME = '<node_name_unspecified>'
UNSPECIFIED_NODE_NAMESPACE = '<node_namespace_unspecified>'

def __init__(
self, *,
executable: Optional[SomeSubstitutionsType] = None,
node_executable: Optional[SomeSubstitutionsType] = None,
package: Optional[SomeSubstitutionsType] = None,
name: Optional[SomeSubstitutionsType] = None,
namespace: Optional[SomeSubstitutionsType] = '',
namespace: Optional[SomeSubstitutionsType] = None,
node_name: Optional[SomeSubstitutionsType] = None,
node_namespace: SomeSubstitutionsType = '',
node_namespace: SomeSubstitutionsType = None,
exec_name: Optional[SomeSubstitutionsType] = None,
parameters: Optional[SomeParameters] = None,
remappings: Optional[SomeRemapRules] = None,
Expand Down Expand Up @@ -215,8 +218,8 @@ def __init__(
self.__remappings = [] if remappings is None else remappings
self.__arguments = arguments

self.__expanded_node_name = '<node_name_unspecified>'
self.__expanded_node_namespace = ''
self.__expanded_node_name = self.UNSPECIFIED_NODE_NAME
self.__expanded_node_namespace = self.UNSPECIFIED_NODE_NAMESPACE
self.__final_node_name = None # type: Optional[Text]
self.__expanded_parameter_files = None # type: Optional[List[Text]]
self.__expanded_remappings = None # type: Optional[List[Tuple[Text, Text]]]
Expand Down Expand Up @@ -321,6 +324,10 @@ def node_name(self):
raise RuntimeError("cannot access 'node_name' before executing action")
return self.__final_node_name

def is_node_name_fully_specified(self):
keywords = (self.UNSPECIFIED_NODE_NAME, self.UNSPECIFIED_NODE_NAMESPACE)
return all(x not in self.node_name for x in keywords)

def _create_params_file_from_dict(self, params):
with NamedTemporaryFile(mode='w', prefix='launch_params_', delete=False) as h:
param_file_path = h.name
Expand All @@ -340,19 +347,24 @@ def _perform_substitutions(self, context: LaunchContext) -> None:
context, normalize_to_list_of_substitutions(self.__node_name))
validate_node_name(self.__expanded_node_name)
self.__expanded_node_name.lstrip('/')
self.__expanded_node_namespace = perform_substitutions(
context, normalize_to_list_of_substitutions(self.__node_namespace))
if not self.__expanded_node_namespace.startswith('/'):
base_ns = context.launch_configurations.get('ros_namespace', '')
self.__expanded_node_namespace = (
base_ns + '/' + self.__expanded_node_namespace
).rstrip('/')
if (
self.__expanded_node_namespace != '' and not
self.__expanded_node_namespace.startswith('/')
):
self.__expanded_node_namespace = '/' + self.__expanded_node_namespace
if self.__expanded_node_namespace != '':
expanded_node_namespace = None
if self.__node_namespace is not None:
expanded_node_namespace = perform_substitutions(
context, normalize_to_list_of_substitutions(self.__node_namespace))
base_ns = context.launch_configurations.get('ros_namespace', None)
if base_ns is not None or expanded_node_namespace is not None:
if expanded_node_namespace is None:
expanded_node_namespace = ''
if base_ns is None:
base_ns = ''
if not expanded_node_namespace.startswith('/'):
expanded_node_namespace = (
base_ns + '/' + expanded_node_namespace
).rstrip('/')
if not expanded_node_namespace.startswith('/'):
expanded_node_namespace = '/' + expanded_node_namespace
self.__expanded_node_namespace = expanded_node_namespace
if expanded_node_namespace is not None:
cmd_extension = ['-r', LocalSubstitution("ros_specific_arguments['ns']")]
self.cmd.extend([normalize_to_list_of_substitutions(x) for x in cmd_extension])
validate_namespace(self.__expanded_node_namespace)
Expand All @@ -368,7 +380,7 @@ def _perform_substitutions(self, context: LaunchContext) -> None:
)
raise
self.__final_node_name = ''
if self.__expanded_node_namespace not in ['', '/']:
if self.__expanded_node_namespace != '/':
self.__final_node_name += self.__expanded_node_namespace
self.__final_node_name += '/' + self.__expanded_node_name
# expand parameters too
Expand Down Expand Up @@ -423,7 +435,7 @@ def execute(self, context: LaunchContext) -> Optional[List[Action]]:
context.extend_locals({'ros_specific_arguments': ros_specific_arguments})
ret = super().execute(context)

if '<node_name_unspecified>' not in self.node_name:
if self.is_node_name_fully_specified():
add_node_name(context, self.node_name)
node_name_count = get_node_name_count(context, self.node_name)
if node_name_count > 1:
Expand Down
28 changes: 25 additions & 3 deletions test_launch_ros/test/test_launch_ros/actions/test_push_ros_namespace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ class Config:
def __init__(
self,
*,
node_ns='',
node_name=None,
node_ns=None,
push_ns=None,
expected_ns=None,
second_push_ns=None
):
self.node_name = node_name
self.push_ns = push_ns
self.node_ns = node_ns
self.expected_ns = expected_ns
Expand Down Expand Up @@ -65,8 +67,20 @@ def __init__(
second_push_ns='/absolute_ns',
node_ns='node_ns',
expected_ns='/absolute_ns/node_ns'),
Config(
node_name='my_node',
push_ns='relative_ns',
second_push_ns='/absolute_ns',
node_ns='node_ns',
expected_ns='/absolute_ns/node_ns'),
Config(
node_name='my_node',
node_ns='node_ns',
expected_ns='/node_ns'),
Config(),
Config(push_ns=''),
Config(
push_ns='',
expected_ns='/'),
))
def test_push_ros_namespace(config):
lc = LaunchContext()
Expand All @@ -80,7 +94,15 @@ def test_push_ros_namespace(config):
package='dont_care',
executable='whatever',
namespace=config.node_ns,
name=config.node_name
)
node._perform_substitutions(lc)
expected_ns = config.expected_ns if config.expected_ns is not None else ''
expected_ns = (
config.expected_ns if config.expected_ns is not None else Node.UNSPECIFIED_NODE_NAMESPACE
)
expected_name = (
config.node_name if config.node_name is not None else Node.UNSPECIFIED_NODE_NAME
)
expected_fqn = expected_ns.rstrip('/') + '/' + expected_name
assert expected_ns == node.expanded_node_namespace
assert expected_fqn == node.node_name
2 changes: 1 addition & 1 deletion test_launch_ros/test/test_launch_ros/test_track_node_names.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ def test_launch_node_with_name_without_namespace():
ld = LaunchDescription([node])
context = _launch(ld)
assert get_node_name_count(context, f'{TEST_NODE_NAMESPACE}/{TEST_NODE_NAME}') == 0
assert get_node_name_count(context, f'/{TEST_NODE_NAME}') == 1
assert get_node_name_count(context, f'/{TEST_NODE_NAME}') == 0


def test_launch_composable_node_with_names(pytestconfig):
Expand Down