Skip to content
Open
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
16 changes: 15 additions & 1 deletion launch_ros/launch_ros/actions/load_composable_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
from launch_ros.parameter_descriptions import ParameterFile

import lifecycle_msgs.msg
import rcl_interfaces.msg

from .composable_node_container import ComposableNodeContainer
from .lifecycle_transition import LifecycleTransition
Expand Down Expand Up @@ -321,7 +322,6 @@ def get_composable_node_load_request(
combined_ns = make_namespace_absolute(prefix_namespace(base_ns, expanded_ns))
if combined_ns is not None:
request.node_namespace = combined_ns
# request.log_level = perform_substitutions(context, node_description.log_level)
remappings = []
global_remaps = context.launch_configurations.get('ros_remaps', None)
if global_remaps:
Expand Down Expand Up @@ -366,4 +366,18 @@ def get_composable_node_load_request(
)
)
]

if composable_node_description.log_level is not None:
log_level_str = perform_substitutions(
context, composable_node_description.log_level).upper()
log_level_map = {
'DEBUG': rcl_interfaces.msg.Log.DEBUG,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be rcl_interfaces.msg.LoggerLevel.

'INFO': rcl_interfaces.msg.Log.INFO,
'WARN': rcl_interfaces.msg.Log.WARN,
'WARNING': rcl_interfaces.msg.Log.WARN,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstring does not mention about this string though... we should add it to the help docstring?

'ERROR': rcl_interfaces.msg.Log.ERROR,
'FATAL': rcl_interfaces.msg.Log.FATAL,
}
request.log_level = log_level_map.get(log_level_str, 0)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fallback is NOT great for the user application. if user sets typos log_level='debig', it silently sets it to UNSET and the user will not be notified configuration gone wrong. i think that this behavior is actually worse then raising the exception, what do you think? i say, we should enforce the check and return error would be better. and again this should be done with rclpy get_logging_severity_from_string?


return request
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,8 @@ def remappings(self) -> Optional[RemapRules]:
def extra_arguments(self) -> Optional[Parameters]:
"""Get container extra arguments YAML files or dicts with substitutions to be performed."""
return super().extra_arguments

@property
def log_level(self) -> Optional[List[Substitution]]:
"""Get log level as a sequence of substitutions to be performed."""
return super().log_level
15 changes: 15 additions & 0 deletions launch_ros/launch_ros/descriptions/composable_node.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ def __init__(
remappings: Optional[SomeRemapRules] = None,
extra_arguments: Optional[SomeParameters] = None,
condition: Optional[Condition] = None,
log_level: Optional[SomeSubstitutionsType] = None,
) -> None:
"""
Initialize a ComposableNode description.
Expand All @@ -57,6 +58,7 @@ def __init__(
:param remappings: list of from/to pairs for remapping names
:param extra_arguments: container specific arguments to be passed to the loaded node
:param condition: action will be executed if the condition evaluates to true
:param log_level: log level for the node (e.g. 'debug', 'info', 'warn', 'error', 'fatal')
"""
self.__package = normalize_to_list_of_substitutions(package)
self.__node_plugin = normalize_to_list_of_substitutions(plugin)
Expand All @@ -83,6 +85,10 @@ def __init__(

self.__condition = condition

self.__log_level = None # type: Optional[List[Substitution]]
if log_level is not None:
self.__log_level = normalize_to_list_of_substitutions(log_level)

@classmethod
def parse(cls, parser: Parser, entity: Entity):
"""Parse composable_node."""
Expand Down Expand Up @@ -138,6 +144,10 @@ def parse(cls, parser: Parser, entity: Entity):
for extra_arg in extra_arguments:
extra_arg.assert_entity_completely_parsed()

log_level = entity.get_attr('log_level', optional=True)
if log_level is not None:
kwargs['log_level'] = parser.parse_substitution(log_level)

return cls, kwargs

@property
Expand Down Expand Up @@ -178,3 +188,8 @@ def extra_arguments(self) -> Optional[Parameters]:
def condition(self) -> Optional[Condition]:
"""Getter for condition."""
return self.__condition

@property
def log_level(self) -> Optional[List[Substitution]]:
"""Get log level as a sequence of substitutions to be performed."""
return self.__log_level