ros2 / launch

Tools for launching multiple processes and for writing tests involving multiple processes.
Apache License 2.0
126 stars 141 forks source link

Add boolean substitutions #598

Closed kenji-miyake closed 2 years ago

kenji-miyake commented 2 years ago

Considering the following use cases, I think it's useful to add boolean substitutions.

<launch>
  <arg name="condition_1" />
  <arg name="condition_2" />

  <!-- Without this PR (if I understand correctly) -->
  <let name="and" value="$(eval '\'$(var condition_1)\' and \'$(var condition_2)\'')" />
  <group if="$(var and)">
    <log message="do something" />
  </group>

  <!-- With this PR -->
  <group if="$(and $(var condition_1) $(var condition_2))">
    <log message="do something" />
  </group>
</launch>

Of course, in this case we can change the arg like this:

<launch>
  <arg name="condition_1_and_condition_2" />

  <group if="$(var condition_1_and_condition_2)">
    <log message="do something" />
  </group>
</launch>

However, I think it doesn't work with the following case:

// caller.launch.xml
<launch>
  <arg name="condition_1" />
  <arg name="condition_2" />

  <group if="$(var condition_1)">
    <log message="do something 1" />
  </group>

  <group if="$(var condition_2)">
    <log message="do something 2" />
  </group>

  <let name="and" value="$(eval '\'$(var condition_1)\' and \'$(var condition_2)\'')" />
  <include file="called.launch.xml">
    <arg name="do_something" value="$(var and)" />
  <include />
</launch>

// called.launch.xml
<launch>
  <arg name="do_something" />

  <group if="$(var do_something)">
    <log message="do something in called.launch.xml" />
  </group>
</launch>
kenji-miyake commented 2 years ago

@adityapande-1995 @hidmic Hello, could you kindly review this, please? :pray:

kenji-miyake commented 2 years ago

As an alternative approach, I guess we can consider handling !, &&, || in IfCondition.

kenji-miyake commented 2 years ago

Mm... I'm not sure why the test failed. :thinking:

$ colcon test --event-handlers console_cohesion+
=================================== FAILURES ===================================
_____________________ test_invalid_parser_implementations ______________________
test/launch/frontend/test_parser.py:64: in test_invalid_parser_implementations
    assert(caught_warnings)
E   assert []
$ colcon test-result --verbose
build/launch/pytest.xml: 306 tests, 0 errors, 1 failure, 0 skipped
- launch.test.launch.frontend.test_parser test_invalid_parser_implementations
  <<< failure message
    assert []
  >>>
build/launch_testing/pytest.xml: 90 tests, 0 errors, 3 failures, 0 skipped
- launch_testing.test.launch_testing.examples.parameters_launch_test parameters_launch_test
  <<< failure message
    parameters_launch_test.TestProcessOutput.test_process_outputs_expected_value[flag1] failed
  >>>
- launch_testing.test.launch_testing.examples.ready_action_test ready_action_test
  <<< failure message
    ready_action_test.TestGoodProcess.test_count_to_four failed
    ready_action_test.TestProcessOutput.test_exit_code failed
    ready_action_test.TestProcessOutput.test_full_output failed
    ready_action_test.TestProcessOutput.test_out_of_order failed
  >>>
- launch_testing.test.launch_testing.examples.terminating_proc_launch_test terminating_proc_launch_test
  <<< failure message
    terminating_proc_launch_test.TestTerminatingProc.test_no_args failed
    terminating_proc_launch_test.TestTerminatingProc.test_unhandled_exception failed
    terminating_proc_launch_test.TestTerminatingProc.test_with_args failed
  >>>
$ pytest -s launch/test/launch/frontend/test_parser.py
================================================================================ test session starts ================================================================================
platform linux -- Python 3.8.10, pytest-6.2.5, py-1.10.0, pluggy-1.0.0
rootdir: /home/kenji/tmp/launch/launch, configfile: pytest.ini
plugins: launch-testing-ros-0.17.0, launch-pytest-0.21.0, launch-testing-0.21.0, ament-pep257-0.11.4, ament-lint-0.11.4, ament-xmllint-0.11.4, ament-copyright-0.11.4, ament-flake8-0.11.4, cov-2.12.1, asyncio-0.15.1, rerunfailures-10.1, repeat-0.9.1, lazy-fixture-0.6.3, datadir-1.3.1, dash-2.0.0, mock-1.10.4, colcon-core-0.7.1
collected 2 items                                                                                                                                                                   

launch/test/launch/frontend/test_parser.py ..

================================================================================= 2 passed in 0.01s =================================================================================
kenji-miyake commented 2 years ago

Actually, this was wrong and only works with limited cased. For example, "true" and "false" works but "false" and "true" doens't.

So we have to write in a more complicated way or use True/False instead of true/false or 1/0.

kenji-miyake commented 2 years ago

The cause of the error was root_entity, parser = Parser.load(file), which seems to affect globally.

kenji-miyake commented 2 years ago

I've added the auto-expansion in 8bdf336fb2fac003a587ea27add46f6e38c2208a. Is it allowed? :thinking:

kenji-miyake commented 2 years ago

~If #600 is accepted, this PR might be replaced by it.~

jacobperron commented 2 years ago

Re-triggering CI with latest changes (and including launch_yaml):

kenji-miyake commented 2 years ago

@jacobperron Hello, can this be merged?

hidmic commented 2 years ago

Alright, I think this is ready to go. Thank you for contribution and patience @kenji-miyake.