launch icon indicating copy to clipboard operation
launch copied to clipboard

Substitutions do not work with `RegisterEventHandler`?

Open galou opened this issue 4 years ago • 6 comments

Bug report

Required Info:

  • Operating System:
    • Ubuntu 20.04
  • Installation type:
    • package
  • Version or commit hash:
    • 0.10.5-1focal.20210423.000156
  • DDS implementation:
    • default
  • Client library (if applicable):
    • N/A

Steps to reproduce issue

cd $ROS_WORKSPACE
git clone https://github.com/galou/launch_bug.git src/launch_bug
colcon build
ros2 launch launch_bug delayed_launch.launch.py

Expected behavior

empty.launch.py should be launched after 1s.

Actual behavior

[INFO] [pause_node-1]: process started with pid [134112]
[INFO] [pause_node-1]: process has finished cleanly [pid 134112]
Task exception was never retrieved
future: <Task finished name='Task-7' coro=<LaunchService._process_one_event() done, defined at /opt/ros/foxy/lib/python3.8/site-packages/launch/launch_service.py:274> exception=SubstitutionFailure('ThisLaunchFileDir used outside of a launch file (in a script)')>
Traceback (most recent call last):
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/launch_service.py", line 276, in _process_one_event
    await self.__process_event(next_event)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/launch_service.py", line 296, in __process_event
    visit_all_entities_and_collect_futures(entity, self.__context))
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/utilities/visit_all_entities_and_collect_futures_impl.py", line 38, in visit_all_entities_and_collect_futures
    sub_entities = entity.visit(context)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/action.py", line 108, in visit
    return self.execute(context)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/actions/include_launch_description.py", line 125, in execute
    launch_description = self.__launch_description_source.get_launch_description(context)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/launch_description_source.py", line 81, in get_launch_description
    perform_substitutions(context, self.__location)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/utilities/perform_substitutions_impl.py", line 26, in perform_substitutions
    return ''.join([context.perform_substitution(sub) for sub in subs])
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/utilities/perform_substitutions_impl.py", line 26, in <listcomp>
    return ''.join([context.perform_substitution(sub) for sub in subs])
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/launch_context.py", line 197, in perform_substitution
    return substitution.perform(self)
  File "/opt/ros/foxy/lib/python3.8/site-packages/launch/substitutions/this_launch_file_dir.py", line 56, in perform
    raise SubstitutionFailure(
launch.substitutions.substitution_failure.SubstitutionFailure: ThisLaunchFileDir used outside of a launch file (in a script)

Additional information

Defining the launch description without event handler as:

    launch_description = LaunchDescription(
        [
            pause_node,
            included_launch_description,
        ],
    )

and removing the event handler works.

galou avatar Jun 29 '21 07:06 galou

There are similarities between this issue and https://github.com/ros2/launch/issues/501. Upon including the root LaunchDescription, the current_launch_file_directory local that ThisLaunchFileDir() needs is properly set but it is not carried over to the context of the nested event handler.

@ivanpauno @wjwwood iterating on https://github.com/ros2/launch/issues/501#issuecomment-814204191, I think we need support for closures.

hidmic avatar Jun 29 '21 13:06 hidmic

There are similarities between this issue and #501. Upon including the root LaunchDescription, the current_launch_file_directory local that ThisLaunchFileDir() needs is properly set but it is not carried over to the context of the nested event handler.

IMHO substitutions in an event hander should be evaluated in the context of the parent action. Maybe, we could even eagerly resolve them when registring the event handler.

ivanpauno avatar Jun 29 '21 15:06 ivanpauno

IMHO substitutions in an event hander should be evaluated in the context of the parent action.

Agreed.

Maybe, we could even eagerly resolve them when registring the event handler.

Can we do that, realistically? An event handler could include an arbitrarily deep launch description.

hidmic avatar Jun 29 '21 15:06 hidmic

I think it would be better to capture the context when the event handler action is handled and use that during the execution of the event handler, but maybe we could "eagerly" process substitutions instead. I'd have to see some prototypes to see what makes sense.

wjwwood avatar Jul 01 '21 05:07 wjwwood

I had a similar problem with the substiitutions that are inside the on_exit list from ExecuteProcess actions.

I was able to replicate the problem using TimerAction. In my case it stop working when everything is inside a GroupAction.

I placed some small launch file that can reproduce the problem. launch_files.zip

Operating System: Ubuntu 20.04

Installation type: package

Ros distro: Galactic

arslogavitabrevis avatar Nov 02 '22 18:11 arslogavitabrevis

I think it would be better to capture the context when the event handler action is handled and use that during the execution of the event handler

Yes, that seems to be the more reasonable solution, as evaluating substitutions early seems more complex to do.

ivanpauno avatar Nov 08 '22 17:11 ivanpauno