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.

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

Add a way to set remapping rules for all nodes in the same scope #163

Merged
merged 4 commits into from
Jul 24, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions launch_ros/launch_ros/actions/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
from .node import Node
from .push_ros_namespace import PushRosNamespace
from .set_parameter import SetParameter
from .set_remap import SetRemap


__all__ = [
'ComposableNodeContainer',
Expand All @@ -28,4 +30,5 @@
'Node',
'PushRosNamespace',
'SetParameter',
'SetRemap',
]
17 changes: 11 additions & 6 deletions launch_ros/launch_ros/actions/load_composable_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,17 @@ def get_composable_node_load_request(
if combined_ns is not None:
request.node_namespace = combined_ns
# request.log_level = perform_substitutions(context, node_description.log_level)
if composable_node_description.remappings is not None:
for from_, to in composable_node_description.remappings:
request.remap_rules.append('{}:={}'.format(
perform_substitutions(context, list(from_)),
perform_substitutions(context, list(to)),
))
remappings = []
global_remaps = context.launch_configurations.get('ros_remaps', None)
ivanpauno marked this conversation as resolved.
Show resolved Hide resolved
if global_remaps:
remappings.extend([f'{src}:={dst}' for src, dst in global_remaps])
if composable_node_description.remappings:
remappings.extend([
f'{perform_substitutions(context, src)}:={perform_substitutions(context, dst)}'
for src, dst in composable_node_description.remappings
])
if remappings:
request.remap_rules = remappings
global_params = context.launch_configurations.get('ros_params', None)
parameters = []
if global_params is not None:
Expand Down
42 changes: 20 additions & 22 deletions launch_ros/launch_ros/actions/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import os
import pathlib
from tempfile import NamedTemporaryFile
from typing import cast
from typing import Dict
from typing import Iterable
from typing import List
Expand Down Expand Up @@ -195,14 +194,6 @@ def __init__(
# All elements in the list are paths to files with parameters (or substitutions that
# evaluate to paths), or dictionaries of parameters (fields can be substitutions).
normalized_params = normalize_parameters(parameters)
if remappings is not None:
i = 0
for remapping in normalize_remap_rules(remappings):
k, v = remapping
cmd += ['-r', LocalSubstitution(
"ros_specific_arguments['remaps'][{}]".format(i),
description='remapping {}'.format(i))]
i += 1
# Forward 'exec_name' as to ExecuteProcess constructor
kwargs['name'] = exec_name
super().__init__(cmd=cmd, **kwargs)
Expand All @@ -211,7 +202,7 @@ def __init__(
self.__node_name = name
self.__node_namespace = namespace
self.__parameters = [] if parameters is None else normalized_params
self.__remappings = [] if remappings is None else remappings
self.__remappings = [] if remappings is None else list(normalize_remap_rules(remappings))
self.__arguments = arguments

self.__expanded_node_name = self.UNSPECIFIED_NODE_NAME
Expand Down Expand Up @@ -401,12 +392,21 @@ def _perform_substitutions(self, context: LaunchContext) -> None:
cmd_extension = ['--params-file', f'{param_file_path}']
self.cmd.extend([normalize_to_list_of_substitutions(x) for x in cmd_extension])
# expand remappings too
if self.__remappings is not None:
global_remaps = context.launch_configurations.get('ros_remaps', None)
if global_remaps or self.__remappings:
self.__expanded_remappings = []
for k, v in self.__remappings:
key = perform_substitutions(context, normalize_to_list_of_substitutions(k))
value = perform_substitutions(context, normalize_to_list_of_substitutions(v))
self.__expanded_remappings.append((key, value))
if global_remaps:
self.__expanded_remappings.extend(global_remaps)
if self.__remappings:
self.__expanded_remappings.extend([
(perform_substitutions(context, src), perform_substitutions(context, dst))
for src, dst in self.__remappings
])
if self.__expanded_remappings:
cmd_extension = []
for src, dst in self.__expanded_remappings:
cmd_extension.extend(['-r', f'{src}:={dst}'])
self.cmd.extend([normalize_to_list_of_substitutions(x) for x in cmd_extension])

def execute(self, context: LaunchContext) -> Optional[List[Action]]:
"""
Expand All @@ -422,13 +422,6 @@ def execute(self, context: LaunchContext) -> Optional[List[Action]]:
ros_specific_arguments['name'] = '__node:={}'.format(self.__expanded_node_name)
if self.__expanded_node_namespace != '':
ros_specific_arguments['ns'] = '__ns:={}'.format(self.__expanded_node_namespace)
if self.__expanded_remappings is not None:
ros_specific_arguments['remaps'] = []
for remapping_from, remapping_to in self.__expanded_remappings:
remap_arguments = cast(List[str], ros_specific_arguments['remaps'])
remap_arguments.append(
'{}:={}'.format(remapping_from, remapping_to)
)
context.extend_locals({'ros_specific_arguments': ros_specific_arguments})
ret = super().execute(context)

Expand All @@ -448,3 +441,8 @@ def execute(self, context: LaunchContext) -> Optional[List[Action]]:
def expanded_node_namespace(self):
"""Getter for expanded_node_namespace."""
return self.__expanded_node_namespace

@property
def expanded_remapping_rules(self):
"""Getter for expanded_remappings."""
return self.__expanded_remappings
91 changes: 91 additions & 0 deletions launch_ros/launch_ros/actions/set_remap.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
# Copyright 2020 Open Source Robotics Foundation, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Module for the `SetRemap` action."""

from typing import List

from launch import Action
from launch import Substitution
from launch.frontend import Entity
from launch.frontend import expose_action
from launch.frontend import Parser
from launch.launch_context import LaunchContext
from launch.some_substitutions_type import SomeSubstitutionsType
from launch.utilities import normalize_to_list_of_substitutions
from launch.utilities import perform_substitutions


@expose_action('set_remap')
class SetRemap(Action):
"""
Action that sets a remapping rule in the current context.

This remapping rule will be passed to all the nodes launched in the same scope, overriding
the ones specified in the `Node` action constructor.
e.g.:
```python3
LaunchDescription([
...,
GroupAction(
actions = [
...,
SetRemap(src='asd', dst='bsd'),
...,
Node(...), // the remap rule will be passed to this node
...,
]
),
Node(...), // here it won't be passed, as it's not in the same scope
...
])
```
"""

def __init__(
self,
src: SomeSubstitutionsType,
dst: SomeSubstitutionsType,
**kwargs
) -> None:
"""Create a SetRemap action."""
super().__init__(**kwargs)
self.__src = normalize_to_list_of_substitutions(src)
self.__dst = normalize_to_list_of_substitutions(dst)

@classmethod
def parse(cls, entity: Entity, parser: Parser):
"""Return `SetRemap` action and kwargs for constructing it."""
_, kwargs = super().parse(entity, parser)
kwargs['src'] = parser.parse_substitution(entity.get_attr('from'))
kwargs['dst'] = parser.parse_substitution(entity.get_attr('to'))
return cls, kwargs

@property
def src(self) -> List[Substitution]:
"""Getter for src."""
return self.__src

@property
def dst(self) -> List[Substitution]:
"""Getter for dst."""
return self.__dst

def execute(self, context: LaunchContext):
"""Execute the action."""
src = perform_substitutions(context, self.__src)
dst = perform_substitutions(context, self.__dst)
global_remaps = context.launch_configurations.get('ros_remaps', [])
global_remaps.append((src, dst))
context.launch_configurations['ros_remaps'] = global_remaps
106 changes: 106 additions & 0 deletions test_launch_ros/test/test_launch_ros/actions/test_set_remap.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
# Copyright 2020 Open Source Robotics Foundation, Inc.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

"""Tests for the SetRemap Action."""

from launch import LaunchContext
from launch.actions import PopLaunchConfigurations
from launch.actions import PushLaunchConfigurations

from launch_ros.actions import Node
from launch_ros.actions import SetRemap
from launch_ros.actions.load_composable_nodes import get_composable_node_load_request
from launch_ros.descriptions import ComposableNode

import pytest


class MockContext:

def __init__(self):
self.launch_configurations = {}

def perform_substitution(self, sub):
return sub.perform(None)


def get_set_remap_test_remaps():
return [
pytest.param(
[('from', 'to')],
id='One remapping rule'
),
pytest.param(
[('from1', 'to1'), ('from2', 'to2')],
id='Two remapping rules'
),
]


@pytest.mark.parametrize(
'remapping_rules',
get_set_remap_test_remaps()
)
def test_set_remap(remapping_rules):
lc = MockContext()
for src, dst in remapping_rules:
SetRemap(src, dst).execute(lc)
assert lc.launch_configurations == {'ros_remaps': remapping_rules}


def test_set_remap_is_scoped():
lc = LaunchContext()
push_conf = PushLaunchConfigurations()
pop_conf = PopLaunchConfigurations()
set_remap = SetRemap('from', 'to')

push_conf.execute(lc)
set_remap.execute(lc)
assert lc.launch_configurations == {'ros_remaps': [('from', 'to')]}
pop_conf.execute(lc)
assert lc.launch_configurations == {}


def test_set_remap_with_node():
lc = MockContext()
node = Node(
package='asd',
executable='bsd',
name='my_node',
namespace='my_ns',
remappings=[('from2', 'to2')]
)
set_remap = SetRemap('from1', 'to1')
hidmic marked this conversation as resolved.
Show resolved Hide resolved
set_remap.execute(lc)
node._perform_substitutions(lc)
assert len(node.expanded_remapping_rules) == 2
assert node.expanded_remapping_rules == [('from1', 'to1'), ('from2', 'to2')]


def test_set_remap_with_composable_node():
lc = MockContext()
node_description = ComposableNode(
package='asd',
plugin='my_plugin',
name='my_node',
namespace='my_ns',
remappings=[('from2', 'to2')]
)
set_remap = SetRemap('from1', 'to1')
set_remap.execute(lc)
request = get_composable_node_load_request(node_description, lc)
remappings = request.remap_rules
assert len(remappings) == 2
assert remappings[0] == 'from1:=to1'
assert remappings[1] == 'from2:=to2'