Skip to content

Commit

Permalink
Remove constructors arguments deprecated since Foxy (#190)
Browse files Browse the repository at this point in the history
Signed-off-by: Ivan Santiago Paunovic <[email protected]>
  • Loading branch information
ivanpauno authored Oct 27, 2020
1 parent 0278d00 commit a8c12a9
Show file tree
Hide file tree
Showing 5 changed files with 2 additions and 150 deletions.
18 changes: 1 addition & 17 deletions launch_ros/launch_ros/actions/lifecycle_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
from typing import cast
from typing import List
from typing import Optional
import warnings

import launch
from launch import SomeSubstitutionsType
Expand All @@ -42,9 +41,8 @@ class LifecycleNode(Node):
def __init__(
self,
*,
name: Optional[SomeSubstitutionsType] = None,
name: SomeSubstitutionsType,
namespace: SomeSubstitutionsType,
node_name: Optional[SomeSubstitutionsType] = None,
**kwargs
) -> None:
"""
Expand All @@ -70,24 +68,10 @@ def __init__(
one, or even all lifecycle nodes, and it requests the targeted nodes
to change state, see its documentation for more details.
.. deprecated:: Foxy
The parameter `node_name` is deprecated, use `name` instead.
:param name: The name of the lifecycle node.
Although it defaults to None it is a required parameter and the default will be removed
in a future release.
:param node_name: (DEPRECATED) The name fo the lifecycle node.
"""
if node_name is not None:
warnings.warn("The parameter 'node_name' is deprecated, use 'name' instead")
if name is not None:
raise RuntimeError(
"Passing both 'node_name' and 'name' parameters. Only use 'name'."
)
name = node_name
# TODO(jacobperron): Remove default value and this check when deprecated API is removed
if name is None:
raise RuntimeError("'name' must not be None.'")
super().__init__(name=name, namespace=namespace, **kwargs)
self.__logger = launch.logging.get_logger(__name__)
self.__rclpy_subscription = None
Expand Down
44 changes: 0 additions & 44 deletions launch_ros/launch_ros/actions/node.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@
from typing import Tuple # noqa: F401
from typing import Union

import warnings

from launch.action import Action
from launch.actions import ExecuteProcess
from launch.frontend import Entity
Expand Down Expand Up @@ -72,12 +70,9 @@ class Node(ExecuteProcess):
def __init__(
self, *,
executable: Optional[SomeSubstitutionsType] = None,
node_executable: Optional[SomeSubstitutionsType] = None,
package: Optional[SomeSubstitutionsType] = None,
name: Optional[SomeSubstitutionsType] = None,
namespace: Optional[SomeSubstitutionsType] = None,
node_name: Optional[SomeSubstitutionsType] = None,
node_namespace: SomeSubstitutionsType = None,
exec_name: Optional[SomeSubstitutionsType] = None,
parameters: Optional[SomeParameters] = None,
remappings: Optional[SomeRemapRules] = None,
Expand Down Expand Up @@ -131,39 +126,19 @@ def __init__(
passed in in order to the node (where the last definition of a
parameter takes effect).
.. deprecated:: Foxy
Parameters `node_executable`, `node_name`, and `node_namespace` are deprecated.
Use `executable`, `name`, and `namespace` instead.
:param: executable the name of the executable to find if a package
is provided or otherwise a path to the executable to run.
:param: node_executable (DEPRECATED) the name of the executable to find if a package
is provided or otherwise a path to the executable to run.
:param: package the package in which the node executable can be found
:param: name the name of the node
:param: namespace the ROS namespace for this Node
:param: exec_name the label used to represent the process.
Defaults to the basename of node executable.
:param: node_name (DEPRECATED) the name of the node
:param: node_namespace (DEPRECATED) the ros namespace for this Node
:param: parameters list of names of yaml files with parameter rules,
or dictionaries of parameters.
:param: remappings ordered list of 'to' and 'from' string pairs to be
passed to the node as ROS remapping rules
:param: arguments list of extra arguments for the node
"""
if node_executable is not None:
warnings.warn(
"The parameter 'node_executable' is deprecated, use 'executable' instead",
stacklevel=2
)
if executable is not None:
raise RuntimeError(
"Passing both 'node_executable' and 'executable' parameters. "
"Only use 'executable'"
)
executable = node_executable

if package is not None:
cmd = [ExecutableInPackage(package=package, executable=executable)]
else:
Expand All @@ -172,28 +147,9 @@ def __init__(
# Reserve space for ros specific arguments.
# The substitutions will get expanded when the action is executed.
cmd += ['--ros-args'] # Prepend ros specific arguments with --ros-args flag
if node_name is not None:
warnings.warn(
"The parameter 'node_name' is deprecated, use 'name' instead",
stacklevel=2)
if name is not None:
raise RuntimeError(
"Passing both 'node_name' and 'name' parameters. Only use 'name'."
)
cmd += ['-r', LocalSubstitution(
"ros_specific_arguments['name']", description='node name')]
name = node_name
if name is not None:
cmd += ['-r', LocalSubstitution(
"ros_specific_arguments['name']", description='node name')]
if node_namespace:
warnings.warn("The parameter 'node_namespace' is deprecated, use 'namespace' instead")
if namespace:
raise RuntimeError(
"Passing both 'node_namespace' and 'namespace' parameters. "
"Only use 'namespace'."
)
namespace = node_namespace
if parameters is not None:
ensure_argument_type(parameters, (list), 'parameters', 'Node')
# All elements in the list are paths to files with parameters (or substitutions that
Expand Down
36 changes: 0 additions & 36 deletions launch_ros/launch_ros/descriptions/composable_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

from typing import List
from typing import Optional
import warnings

from launch.some_substitutions_type import SomeSubstitutionsType
from launch.substitution import Substitution
Expand All @@ -39,56 +38,21 @@ def __init__(
plugin: Optional[SomeSubstitutionsType] = None,
name: Optional[SomeSubstitutionsType] = None,
namespace: Optional[SomeSubstitutionsType] = None,
node_plugin: Optional[SomeSubstitutionsType] = None,
node_name: Optional[SomeSubstitutionsType] = None,
node_namespace: Optional[SomeSubstitutionsType] = None,
parameters: Optional[SomeParameters] = None,
remappings: Optional[SomeRemapRules] = None,
extra_arguments: Optional[SomeParameters] = None,
) -> None:
"""
Initialize a ComposableNode description.
.. deprecated:: Foxy
Parameters `node_plugin`, `node_name`, and `node_namespace` are deprecated.
Use `plugin`, `name`, and `namespace` instead.
:param package: name of the ROS package the node plugin lives in
:param plugin: name of the plugin to be loaded
:param name: name to give to the ROS node
:param namespace: namespace to give to the ROS node
:param node_plugin: (DEPRECATED) name of the plugin to be loaded
:param node_name: (DEPRECATED) name the node should have
:param node_namespace: (DEPRECATED) namespace the node should create topics/services/etc in
:param parameters: list of either paths to yaml files or dictionaries of parameters
:param remappings: list of from/to pairs for remapping names
:param extra_arguments: container specific arguments to be passed to the loaded node
"""
if node_plugin is not None:
warnings.warn("The parameter 'node_plugin' is deprecated, use 'plugin' instead")
if plugin is not None:
raise RuntimeError(
"Passing both 'node_plugin' and 'plugin' parameters. Only use 'plugin'."
)
plugin = node_plugin
if plugin is None:
raise RuntimeError("The 'plugin' parameter is required")
if node_name is not None:
warnings.warn("The parameter 'node_name' is deprecated, use 'name' instead")
if name is not None:
raise RuntimeError(
"Passing both 'node_name' and 'name' parameters. Only use 'name'."
)
name = node_name
if node_namespace is not None:
warnings.warn("The parameter 'node_namespace' is deprecated, use 'namespace' instead")
if namespace is not None:
raise RuntimeError(
"Passing both 'node_namespace' and 'namespace' parameters. "
"Only use 'namespace'."
)
namespace = node_namespace

self.__package = normalize_to_list_of_substitutions(package)
self.__node_plugin = normalize_to_list_of_substitutions(plugin)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ def test_lifecycle_node_constructor():
executable='bsd',
name='my_node',
)
# Construction without name
# TODO(ivanpauno): This should raise TypeError.
with pytest.raises(RuntimeError):
with pytest.raises(TypeError):
LifecycleNode(
package='asd',
executable='bsd',
Expand Down
50 changes: 0 additions & 50 deletions test_launch_ros/test/test_launch_ros/actions/test_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import pathlib
from typing import List
import unittest
import warnings

from launch import LaunchContext
from launch import LaunchDescription
Expand Down Expand Up @@ -250,55 +249,6 @@ def test_create_node_with_invalid_parameters(self):
)
self._assert_launch_no_errors([node_action])

def test_deprecated_node_parameters(self):
"""Test that using deprecated parameters will warn/error."""
# Note that the following tests are expected to break when the deprecated
# parameters are eventually removed and should be updated accordingly.
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
node_action = launch_ros.actions.Node(
package='demo_nodes_py', node_executable='talker_qos', output='screen',
node_name='my_node', node_namespace='my_ns'
)
self._assert_launch_no_errors([node_action])
self.assertEqual(len(w), 3)
self.assertTrue(issubclass(w[0].category, UserWarning))
self.assertTrue(issubclass(w[1].category, UserWarning))
self.assertTrue(issubclass(w[2].category, UserWarning))

# Providing both 'node_executable' and 'executable' should throw
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
with self.assertRaises(RuntimeError):
launch_ros.actions.Node(
package='demo_nodes_py', node_executable='talker_qos', executable='talker_qos',
output='screen', name='my_node', namespace='my_ns'
)
self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[0].category, UserWarning))

# Providing both 'node_name' and 'name' should throw
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
with self.assertRaises(RuntimeError):
launch_ros.actions.Node(
package='demo_nodes_py', executable='talker_qos', output='screen',
node_name='my_node', name='my_node', namespace='my_ns'
)
self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[0].category, UserWarning))

# Providing both 'node_namespace' and 'namespace' should throw
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
with self.assertRaises(RuntimeError):
launch_ros.actions.Node(
package='demo_nodes_py', executable='talker_qos', output='screen',
name='my_node', node_namespace='my_ns', namespace='my_ns'
)
self.assertEqual(len(w), 1)
self.assertTrue(issubclass(w[0].category, UserWarning))

def test_launch_node_with_invalid_parameter_dicts(self):
"""Test launching a node with invalid parameter dicts."""
# Substitutions aren't expanded until the node action is executed, at which time a type
Expand Down

0 comments on commit a8c12a9

Please sign in to comment.