Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

twister: support segger real-time-transfer (rtt) for serial_pty via west #81837

Closed
wants to merge 4 commits into from

Conversation

cfriedt
Copy link
Member

@cfriedt cfriedt commented Nov 24, 2024

Add initial support for RTT (Segger Real-time Transfer) for reading console messages with twister via west when testing with hardware.

Note

This support should be considered experimental as transfer speed, buffering, buffer size, and buffer count are all tuneable parameters that can vary per-platform and debug adapter. Physical UARTs or other higher-speed I/O interfaces are certainly still the primary means of capturing test output from twister when connected to hardware.

Tested with:

twister -p nucleo_l496zg --device-testing --flash-before --west-runner openocd --west-flash --device-serial-pty rtt -T samples/hello_world
cat twister-out/nucleo_l496zg_stm32l496xx/samples/hello_world/sample.basic.helloworld/handler.log
*** Booting Zephyr OS build v4.0.0-749-gc9e567da3747 ***
Hello World! nucleo_l496zg/stm32l496xx

This change set also includes updates to jlink and openocd west runners to add an --rtt-quiet option. The new option suppresses the stdout and stderr of subprocesses (e.g. JLinkRTTLogger or openocd), which is the expectation for --device-serial-pty scripts. By suppressing unnecessary output, we can leverage west rtt in an automatically generated shell script (as opposed to importing or duplicating more python code in twister).

Currently, the only other RTT-capable west runner is pyocd.

Additionally, the DeviceHandler.handle() method of handler.py is a bit tricky to work with; I was unsuccessful in making separate methods for _do_flash() and _do_serial() portions of the operation. That would have simplified the --flash-before logic. Python was throwing exceptions claiming that code was attempting to adjust objects shared between processes without synchronization. At some point, the handle() method should probably be refactored, but it was considered beyond what was necessary for the first draft of this PR.

@cfriedt cfriedt force-pushed the twister-rtt-support branch 2 times, most recently from f0f5565 to f71d4f1 Compare November 24, 2024 18:35
@cfriedt
Copy link
Member Author

cfriedt commented Nov 24, 2024

@topisani - it would be great if you could try this out as well.

@cfriedt
Copy link
Member Author

cfriedt commented Nov 24, 2024

Oh wow - the bot made a lot of review requests 😅

@cfriedt cfriedt force-pushed the twister-rtt-support branch from f71d4f1 to 04de997 Compare November 26, 2024 17:22
@cfriedt
Copy link
Member Author

cfriedt commented Nov 26, 2024

  • switched to using the already-resolved runner variable instead of pulling it out of self.options.west_runner exclusively, since the --hardware-map option can also be used to provide the runner

It would be good to also parse the --west-flash argument (if it exists) and the runner_params field of the hardware map file to extract runner arguments.

@cfriedt cfriedt force-pushed the twister-rtt-support branch from 04de997 to 9d346ad Compare November 26, 2024 17:31
@cfriedt
Copy link
Member Author

cfriedt commented Nov 26, 2024

  • replaced self.rtt_port = rtt_quiet with self.rtt_quiet = rtt_quiet in jlink.py

@cfriedt cfriedt changed the title twister: support segger real-time-transfer (rtt) for serial_pty twister: support segger real-time-transfer (rtt) for serial_pty via west Nov 30, 2024
Add support for the --rtt-quiet argument, which prevents
subprocesses from printing to stdout and stderr, so that rtt
messages are the only items printed to standard output.

This almnost completely silences the 'west rtt' command with the
exception of '-- west rtt: using runner openocd' bring printed
on the first line of the output.

In order to silence that line from west, the top-level '--quiet'
argument must be passed to west. E.g.

west -qqqq rtt --rtt-quiet ...

Signed-off-by: Chris Friedt <[email protected]>
Add support for the --rtt-quiet argument, which prevents
subprocesses from printing to stdout and stderr, so that rtt
messages are the only items printed to standard output.

This almnost completely silences the 'west rtt' command with the
exception of '-- west rtt: using runner openocd' bring printed
on the first line of the output.

In order to silence that line from west, the top-level '--quiet'
argument must be passed to west. E.g.

west -qqqq rtt --rtt-quiet ...

Signed-off-by: Chris Friedt <[email protected]>
Ensure that --device-serial-pty may be used with --flash-before
in order to support capturing test results via RTT console.

This is required (at least) for openocd, since the flash (or
debug or rtt or any other operation) will fail if an existing
process is already in control of the USB device in question.

Signed-off-by: Chris Friedt <[email protected]>
Support RTT (Segger Real-time Transfer) for reading console
messages with twister when testing with hardware.

Tested with:

twister -p nucleo_l496zg --device-testing --flash-before \
  --west-runner openocd --west-flash --device-serial-pty rtt \
  -T samples/hello_world

cat twister-out/nucleo_l496zg_stm32l496xx/samples/hello_world/\
sample.basic.helloworld/handler.log
*** Booting Zephyr OS build v4.0.0-749-gc9e567da3747 ***
Hello World! nucleo_l496zg/stm32l496xx

Signed-off-by: Chris Friedt <[email protected]>
@cfriedt cfriedt force-pushed the twister-rtt-support branch from 9d346ad to 5e91f54 Compare January 10, 2025 18:03
@cfriedt
Copy link
Member Author

cfriedt commented Jan 10, 2025

  • rebased to fix merge conflict

@zephyrbot zephyrbot requested a review from mbolivar January 10, 2025 18:04
Copy link
Collaborator

@hakehuang hakehuang left a comment

Choose a reason for hiding this comment

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

can we update document for this?

@@ -873,6 +873,18 @@ def parse_arguments(
logger.error("west-flash requires device-testing to be enabled")
sys.exit(1)

if options.device_serial_pty and options.device_serial_pty == "rtt":
if options.west_flash is None:
logger.error("--device-serial-pty rtt requires --west-flash")
Copy link
Member

Choose a reason for hiding this comment

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

--device-serial-pty accept a script name, can be anything, here you assume that the script name (is this a script?) is always going to be rtt, or asking that if someone want to use this, they have to call their script rtt?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, in this case rtt is a keyword, and twister simply re-uses west's rtt integration.

Normally, the argument for device-serial-pty is a script through, so that hasn't changed.

sys.exit(1)

# add the following options
options.extra_args += ['CONFIG_USE_SEGGER_RTT=y',
Copy link
Member

Choose a reason for hiding this comment

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

not a fan of injecting HW related kconfigs into twister like this, this should be done on the platform/hardware map/ or test level, not in twister. Using a snippet for example is an option.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've found actually that this works better with a snippet specified in testcase.yml.

I think this can be removed.

@@ -706,6 +707,23 @@ def _get_serial_device(self, serial_pty, hardware_serial):

return serial_device, ser_pty_process

def _create_serial_pty_script(self, runner):
Copy link
Member

Choose a reason for hiding this comment

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

ok, so you are creating the pty script on the fly, if I understand this correctly, it is an abuse of the --device-serial-pty option, we probably need something else to cover this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

"abuse" is probably a strong term.

I prefer "reuse" - we're simply reusing west's rtt integration.

Copy link
Member

Choose a reason for hiding this comment

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

    device.add_argument("--device-serial-pty",
                        help="""Script for controlling pseudoterminal.
                        Twister believes that it interacts with a terminal
                        when it actually interacts with the script.

                        E.g "twister --device-testing
                        --device-serial-pty <script>

Not using a script here is wrong usage. If you want to use a built-in rtt support you are adding, you need to find another way, maybe yet another option that deals with keywords

@cfriedt cfriedt closed this Jan 17, 2025
@nashif
Copy link
Member

nashif commented Jan 17, 2025

why close?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Twister Twister area: West West utility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants