[ros2interface] Allow stdin input for 'ros2 interface show'

[ghost]

Implements #349.

Interface types can now be piped directly into show, like this:
$ ros2 topic type /chatter | ros2 interface show

[dirk-thomas]

I don't think the actually read data from stdin should be the default value. When you call --help the default value would also be different.

Btw calling ros2 interface show results in an exception atm.

Also the usage / help doesn't give any indicator that the type can be piped in.

[ghost]

I've attached an action to add_argument that reads from stdin if - was passed. Requiring - preserves the parser's required argument check on type since we don't have to allow type to be missing. This requires explicitly running

$ ros2 topic type /chatter | ros2 interface show -

rather than

$ ros2 topic type /chatter | ros2 interface show

This also replaces reading stdin directly for the default value and fixes the resulting exceptions.

I've also added more information in the help but I'm not sure if it's written clearly enough, or if there's a convention that I'm breaking.

[dirk-thomas]

Thanks for the updates.

I applies a few changes. Please have a look:

• 0134dd0 Move all the help text into the help attribute. That avoids potentially overwriting the epilog if set externally. Also the textwrap.dedent seems unnecessary.
• a085e27 Fix the import order to pass the linters.
• f14c359 Simplify the logic in the custom action to avoid duplicate parts.
• 3e7241a Handle the case where the piped command fails / doesn't output anything more gracefully.

[dirk-thomas]

@sharminramli It would be great if you could add a unit test to cover this new feature to avoid it regressing in the future.

[dirk-thomas]

It would be great if you could add a unit test to cover this new feature to avoid it regressing in the future.

@sharminramli Friendly ping.

[ghost]

@dirk-thomas Sorry about the delay. Thanks for the changes, they look great.

I started working on the unit tests, but I ran into some issues piping stdin to the test process. I am patching a mock stdin to the test function with

    @patch('sys.stdin', new_callable=StringIO)
    def test_show_message_from_stdin(self, mocked_stdin):
        mocked_stdin.write('std_msgs/msg/String')
        mocked_stdin.seek(0)
        with self.launch_interface_command(
            arguments=['show', '-']
        ) as interface_command:
            assert interface_command.wait_for_shutdown(timeout=2)
        assert interface_command.exit_code == launch_testing.asserts.EXIT_OK
        assert launch_testing.tools.expect_output(
            expected_lines=[
                'string data'
            ],
            text=interface_command.output,
            strict=True
        )

but it appears to hang at
values = sys.stdin.readline().strip() of show.py. Is there something I'm missing?

[hidmic]

@sharminramli you won't be able to pipe data into the launched process like that. launch does not expose the stdin pipe either, so your best bet right now is to use shell=True and pipe text. For that, you'll have to change the test fixture a bit to be able to launch an action like:

command_action = ExecuteProcess(
  cmd=[sys.executable, '-c', "print('std_msgs/msg/String')", '|', 'ros2', 'interface', 'show', '-'],
  shell=True,
  output='screen'
)

Alternatively, you can propose a reasonable way to expose the stdin pipe for launched processes in general (back in the launch repo).

[dirk-thomas]

@sharminramli Friendly ping.

[ghost]

@dirk-thomas Sorry for the wait. I've added the test as test_show_stdin in test_cli.py.
@hidmic Thanks for the help. I'm not familiar with the launch repo as of now, so I've gone with the first option and modified launch_interface_command to accept the shell option and a list of arguments to be prepended.

[hidmic]

LGTM !

• Linux
• Linux-aarch64
• macOS
• Windows

[ghost]

Rebased and changed the test to use test_msgs/msg/Nested rather than std_msgs/msg/String (ros2/common_interfaces#89). The help message should also be updated but it's being modified in #516 as well.

[audrow]

I've tested this branch on my computer and it works well - however, test_show_message fails because of the comments added to the message file in Foxy.

[hidmic]

however, test_show_message fails because of the comments added to the message file in Foxy.

That was fixed in #516. As a result, this now needs a rebase.

[ghost]

Rebased and changed the help message to refer to example_interfaces rather than std_msgs. I'm not sure why the Jenkins build is failing though.

[audrow]

• Linux
• Linux-aarch64
• macOS
• Windows

[hidmic]

Thanks for re-running CI @audrow !

[hidmic]

Alright, thank you for contribution and your patience @sharminramli ! Merging.