From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1E1D443404; Thu, 30 Nov 2023 22:50:09 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AEACA40277; Thu, 30 Nov 2023 22:50:08 +0100 (CET) Received: from mail-pg1-f179.google.com (mail-pg1-f179.google.com [209.85.215.179]) by mails.dpdk.org (Postfix) with ESMTP id 463C740266 for ; Thu, 30 Nov 2023 22:50:06 +0100 (CET) Received: by mail-pg1-f179.google.com with SMTP id 41be03b00d2f7-5c239897895so1212156a12.2 for ; Thu, 30 Nov 2023 13:50:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iol.unh.edu; s=unh-iol; t=1701381005; x=1701985805; darn=dpdk.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=r2rCgaVPIiZDDvjovJ8LFnBCzETgA5UQm3EmLq5LTcE=; b=HHhi3RZizIprWd9SAqWd/tiI3M7Ohxcbw/jnkw6Z1npYHuorysC26XSaCY345NTJWM n012zT7lnJRMNISSrx9u4B3ymAA9pGjnW+GIrz//kwH8zvgpdW/HmptaaThst1peJtp/ blvoH9cWQlC2yr6Q6f9Ik+I9iuNdbwvwDAZsc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701381005; x=1701985805; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=r2rCgaVPIiZDDvjovJ8LFnBCzETgA5UQm3EmLq5LTcE=; b=E5hO771iLuGY11TLA9yvQMbfnH74v0P7GwFileTqRiWd1wnAN6gOWdzCrjWEgiNWG1 kYXKscLxJDO74slTc6rJAXoW6/fpJyYjTdFnQiHyiCZ4ggNIEHy0uTd6WWvxsk+sMIel b9t6pCFjavlVuabZc0BkPVHyalu2csxk0kRi8QcqmhzJkstfKtM6smo4cFQKm1XcrQYI ZlL05oB/dkLJgth0cH9a41iHt3RP4wdTSL0yzgt8XDik913nIibcFFGPZ6F64yY0NmE4 g5cII091RsyL0h1aagN4JN8ARGMiV+sRRW63jCooTdOmQXMH/048mfUd7AguhmYAtW7/ 1buA== X-Gm-Message-State: AOJu0Yw4G8odmioTS0wm+YFV1/mKpCZ0GV81HdDeVbpAWrxfDAap0Ber EcrNa6qsyHUFRft/CyUn1I1dqfg47tREXdpZGjgVXA== X-Google-Smtp-Source: AGHT+IG7r3VaMTVRa1RFtlhBtelRvlQMJ9WyNXgq0iJ8mby6NE386H0or4Q7UiPam6KN81RWkxzf/jCoOaxr11zU2wk= X-Received: by 2002:a17:90b:388e:b0:285:c4f1:4646 with SMTP id mu14-20020a17090b388e00b00285c4f14646mr15518589pjb.46.1701381005003; Thu, 30 Nov 2023 13:50:05 -0800 (PST) MIME-Version: 1.0 References: <20231115130959.39420-1-juraj.linkes@pantheon.tech> <20231123151344.162812-1-juraj.linkes@pantheon.tech> <20231123151344.162812-13-juraj.linkes@pantheon.tech> In-Reply-To: <20231123151344.162812-13-juraj.linkes@pantheon.tech> From: Jeremy Spewock Date: Thu, 30 Nov 2023 16:49:54 -0500 Message-ID: Subject: Re: [PATCH v8 12/21] dts: interactive remote session docstring update To: =?UTF-8?Q?Juraj_Linke=C5=A1?= Cc: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, yoan.picchi@foss.arm.com, Luca.Vizzarro@arm.com, dev@dpdk.org Content-Type: multipart/alternative; boundary="0000000000006e60f2060b65a2be" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --0000000000006e60f2060b65a2be Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, Nov 23, 2023 at 10:14=E2=80=AFAM Juraj Linke=C5=A1 wrote: > Format according to the Google format and PEP257, with slight > deviations. > > Signed-off-by: Juraj Linke=C5=A1 > --- > .../interactive_remote_session.py | 36 +++---- > .../remote_session/interactive_shell.py | 99 +++++++++++-------- > dts/framework/remote_session/python_shell.py | 26 ++++- > dts/framework/remote_session/testpmd_shell.py | 58 +++++++++-- > 4 files changed, 149 insertions(+), 70 deletions(-) > > diff --git a/dts/framework/remote_session/interactive_remote_session.py > b/dts/framework/remote_session/interactive_remote_session.py > index 098ded1bb0..1cc82e3377 100644 > --- a/dts/framework/remote_session/interactive_remote_session.py > +++ b/dts/framework/remote_session/interactive_remote_session.py > @@ -22,27 +22,23 @@ > class InteractiveRemoteSession: > """SSH connection dedicated to interactive applications. > > - This connection is created using paramiko and is a persistent > connection to the > - host. This class defines methods for connecting to the node and > configures this > - connection to send "keep alive" packets every 30 seconds. Because > paramiko attempts > - to use SSH keys to establish a connection first, providing a passwor= d > is optional. > - This session is utilized by InteractiveShells and cannot be > interacted with > - directly. > - > - Arguments: > - node_config: Configuration class for the node you are connecting > to. > - _logger: Desired logger for this session to use. > + The connection is created using `paramiko < > https://docs.paramiko.org/en/latest/>`_ > + and is a persistent connection to the host. This class defines the > methods for connecting > + to the node and configures the connection to send "keep alive" > packets every 30 seconds. > + Because paramiko attempts to use SSH keys to establish a connection > first, providing > + a password is optional. This session is utilized by InteractiveShell= s > + and cannot be interacted with directly. > > Attributes: > - hostname: Hostname that will be used to initialize a connection > to the node. > - ip: A subsection of hostname that removes the port for the > connection if there > + hostname: The hostname that will be used to initialize a > connection to the node. > + ip: A subsection of `hostname` that removes the port for the > connection if there > is one. If there is no port, this will be the same as > hostname. > - port: Port to use for the ssh connection. This will be extracted > from the > - hostname if there is a port included, otherwise it will > default to 22. > + port: Port to use for the ssh connection. This will be extracted > from `hostname` > + if there is a port included, otherwise it will default to > ``22``. > username: User to connect to the node with. > password: Password of the user connecting to the host. This will > default to an > empty string if a password is not provided. > - session: Underlying paramiko connection. > + session: The underlying paramiko connection. > > Raises: > SSHConnectionError: There is an error creating the SSH connectio= n. > @@ -58,9 +54,15 @@ class InteractiveRemoteSession: > _node_config: NodeConfiguration > _transport: Transport | None > > - def __init__(self, node_config: NodeConfiguration, _logger: DTSLOG) > -> None: > + def __init__(self, node_config: NodeConfiguration, logger: DTSLOG) -= > > None: > + """Connect to the node during initialization. > + > + Args: > + node_config: The test run configuration of the node to > connect to. > + logger: The logger instance this session will use. > + """ > self._node_config =3D node_config > - self._logger =3D _logger > + self._logger =3D logger > self.hostname =3D node_config.hostname > self.username =3D node_config.user > self.password =3D node_config.password if node_config.password e= lse > "" > diff --git a/dts/framework/remote_session/interactive_shell.py > b/dts/framework/remote_session/interactive_shell.py > index 4db19fb9b3..b158f963b6 100644 > --- a/dts/framework/remote_session/interactive_shell.py > +++ b/dts/framework/remote_session/interactive_shell.py > @@ -3,18 +3,20 @@ > > """Common functionality for interactive shell handling. > > -This base class, InteractiveShell, is meant to be extended by other > classes that > -contain functionality specific to that shell type. These derived classes > will often > -modify things like the prompt to expect or the arguments to pass into th= e > application, > -but still utilize the same method for sending a command and collecting > output. How > -this output is handled however is often application specific. If an > application needs > -elevated privileges to start it is expected that the method for gaining > those > -privileges is provided when initializing the class. > +The base class, :class:`InteractiveShell`, is meant to be extended by > subclasses that contain > +functionality specific to that shell type. These subclasses will often > modify things like > +the prompt to expect or the arguments to pass into the application, but > still utilize > +the same method for sending a command and collecting output. How this > output is handled however > +is often application specific. If an application needs elevated > privileges to start it is expected > +that the method for gaining those privileges is provided when > initializing the class. > + > +The :option:`--timeout` command line argument and the > :envvar:`DTS_TIMEOUT` > +environment variable configure the timeout of getting the output from > command execution. > """ > > from abc import ABC > from pathlib import PurePath > -from typing import Callable > +from typing import Callable, ClassVar > > from paramiko import Channel, SSHClient, channel # type: ignore[import] > > @@ -30,28 +32,6 @@ class InteractiveShell(ABC): > and collecting input until reaching a certain prompt. All interactiv= e > applications > will use the same SSH connection, but each will create their own > channel on that > session. > - > - Arguments: > - interactive_session: The SSH session dedicated to interactive > shells. > - logger: Logger used for displaying information in the console. > - get_privileged_command: Method for modifying a command to allow > it to use > - elevated privileges. If this is None, the application will > not be started > - with elevated privileges. > - app_args: Command line arguments to be passed to the application > on startup. > - timeout: Timeout used for the SSH channel that is dedicated to > this interactive > - shell. This timeout is for collecting output, so if reading > from the buffer > - and no output is gathered within the timeout, an exception i= s > thrown. > - > - Attributes > - _default_prompt: Prompt to expect at the end of output when > sending a command. > - This is often overridden by derived classes. > - _command_extra_chars: Extra characters to add to the end of ever= y > command > - before sending them. This is often overridden by derived > classes and is > - most commonly an additional newline character. > - path: Path to the executable to start the interactive applicatio= n. > - dpdk_app: Whether this application is a DPDK app. If it is, the > build > - directory for DPDK on the node will be prepended to the path > to the > - executable. > """ > > _interactive_session: SSHClient > @@ -61,10 +41,22 @@ class InteractiveShell(ABC): > _logger: DTSLOG > _timeout: float > _app_args: str > - _default_prompt: str =3D "" > - _command_extra_chars: str =3D "" > - path: PurePath > - dpdk_app: bool =3D False > + > + #: Prompt to expect at the end of output when sending a command. > + #: This is often overridden by subclasses. > + _default_prompt: ClassVar[str] =3D "" > + > + #: Extra characters to add to the end of every command > + #: before sending them. This is often overridden by subclasses and i= s > + #: most commonly an additional newline character. > + _command_extra_chars: ClassVar[str] =3D "" > + > + #: Path to the executable to start the interactive application. > + path: ClassVar[PurePath] > + > + #: Whether this application is a DPDK app. If it is, the build > directory > + #: for DPDK on the node will be prepended to the path to the > executable. > + dpdk_app: ClassVar[bool] =3D False > > def __init__( > self, > @@ -74,6 +66,19 @@ def __init__( > app_args: str =3D "", > timeout: float =3D SETTINGS.timeout, > ) -> None: > + """Create an SSH channel during initialization. > + > + Args: > + interactive_session: The SSH session dedicated to interactiv= e > shells. > + logger: The logger instance this session will use. > + get_privileged_command: A method for modifying a command to > allow it to use > + elevated privileges. If :data:`None`, the application > will not be started > + with elevated privileges. > + app_args: The command line arguments to be passed to the > application on startup. > + timeout: The timeout used for the SSH channel that is > dedicated to this interactive > + shell. This timeout is for collecting output, so if > reading from the buffer > + and no output is gathered within the timeout, an > exception is thrown. > + """ > self._interactive_session =3D interactive_session > self._ssh_channel =3D self._interactive_session.invoke_shell() > self._stdin =3D self._ssh_channel.makefile_stdin("w") > @@ -90,6 +95,10 @@ def _start_application(self, get_privileged_command: > Callable[[str], str] | None > > This method is often overridden by subclasses as their process f= or > starting may look different. > + > + Args: > + get_privileged_command: A function (but could be any > callable) that produces > + the version of the command with elevated privileges. > """ > start_command =3D f"{self.path} {self._app_args}" > if get_privileged_command is not None: > @@ -97,16 +106,24 @@ def _start_application(self, get_privileged_command: > Callable[[str], str] | None > self.send_command(start_command) > > def send_command(self, command: str, prompt: str | None =3D None) -> > str: > - """Send a command and get all output before the expected ending > string. > + """Send `command` and get all output before the expected ending > string. > > Lines that expect input are not included in the stdout buffer, s= o > they cannot > - be used for expect. For example, if you were prompted to log int= o > something > - with a username and password, you cannot expect "username:" > because it won't > - yet be in the stdout buffer. A workaround for this could be > consuming an > - extra newline character to force the current prompt into the > stdout buffer. > + be used for expect. > + > + Example: > + If you were prompted to log into something with a username > and password, > + you cannot expect ``username:`` because it won't yet be in > the stdout buffer. > + A workaround for this could be consuming an extra newline > character to force > + the current `prompt` into the stdout buffer. > + > + Args: > + command: The command to send. > + prompt: After sending the command, `send_command` will be > expecting this string. > + If :data:`None`, will use the class's default prompt. > > Returns: > - All output in the buffer before expected string > + All output in the buffer before expected string. > """ > self._logger.info(f"Sending: '{command}'") > if prompt is None: > @@ -124,8 +141,10 @@ def send_command(self, command: str, prompt: str | > None =3D None) -> str: > return out > > def close(self) -> None: > + """Properly free all resources.""" > self._stdin.close() > self._ssh_channel.close() > > def __del__(self) -> None: > + """Make sure the session is properly closed before deleting the > object.""" > self.close() > diff --git a/dts/framework/remote_session/python_shell.py > b/dts/framework/remote_session/python_shell.py > index cc3ad48a68..ccfd3783e8 100644 > --- a/dts/framework/remote_session/python_shell.py > +++ b/dts/framework/remote_session/python_shell.py > @@ -1,12 +1,32 @@ > # SPDX-License-Identifier: BSD-3-Clause > # Copyright(c) 2023 PANTHEON.tech s.r.o. > > +"""Python interactive shell. > + > +Typical usage example in a TestSuite:: > + > + from framework.remote_session import PythonShell > + python_shell =3D self.tg_node.create_interactive_shell( > + PythonShell, timeout=3D5, privileged=3DTrue > + ) > + python_shell.send_command("print('Hello World')") > + python_shell.close() > +""" > + > from pathlib import PurePath > +from typing import ClassVar > > from .interactive_shell import InteractiveShell > > > class PythonShell(InteractiveShell): > - _default_prompt: str =3D ">>>" > - _command_extra_chars: str =3D "\n" > - path: PurePath =3D PurePath("python3") > + """Python interactive shell.""" > + > + #: Python's prompt. > + _default_prompt: ClassVar[str] =3D ">>>" > + > + #: This forces the prompt to appear after sending a command. > + _command_extra_chars: ClassVar[str] =3D "\n" > + > + #: The Python executable. > + path: ClassVar[PurePath] =3D PurePath("python3") > diff --git a/dts/framework/remote_session/testpmd_shell.py > b/dts/framework/remote_session/testpmd_shell.py > index 08ac311016..79481e845c 100644 > --- a/dts/framework/remote_session/testpmd_shell.py > +++ b/dts/framework/remote_session/testpmd_shell.py > @@ -1,41 +1,79 @@ > # SPDX-License-Identifier: BSD-3-Clause > # Copyright(c) 2023 University of New Hampshire > > Should you add to the copyright here for adding comments? > +"""Testpmd interactive shell. > + > +Typical usage example in a TestSuite:: > + > + testpmd_shell =3D self.sut_node.create_interactive_shell( > + TestPmdShell, privileged=3DTrue > + ) > + devices =3D testpmd_shell.get_devices() > + for device in devices: > + print(device) > + testpmd_shell.close() > +""" > + > from pathlib import PurePath > -from typing import Callable > +from typing import Callable, ClassVar > > from .interactive_shell import InteractiveShell > > > class TestPmdDevice(object): > + """The data of a device that testpmd can recognize. > + > + Attributes: > + pci_address: The PCI address of the device. > + """ > + > pci_address: str > > def __init__(self, pci_address_line: str): > + """Initialize the device from the testpmd output line string. > + > + Args: > + pci_address_line: A line of testpmd output that contains a > device. > + """ > self.pci_address =3D pci_address_line.strip().split(": ")[1].str= ip() > > def __str__(self) -> str: > + """The PCI address captures what the device is.""" > return self.pci_address > > > class TestPmdShell(InteractiveShell): > - path: PurePath =3D PurePath("app", "dpdk-testpmd") > - dpdk_app: bool =3D True > - _default_prompt: str =3D "testpmd>" > - _command_extra_chars: str =3D "\n" # We want to append an extra > newline to every command > + """Testpmd interactive shell. > + > + The testpmd shell users should never use > + the :meth:`~.interactive_shell.InteractiveShell.send_command` method > directly, but rather > + call specialized methods. If there isn't one that satisfies a need, > it should be added. > + """ > + > + #: The path to the testpmd executable. > + path: ClassVar[PurePath] =3D PurePath("app", "dpdk-testpmd") > + > + #: Flag this as a DPDK app so that it's clear this is not a system > app and > + #: needs to be looked in a specific path. > + dpdk_app: ClassVar[bool] =3D True > + > + #: The testpmd's prompt. > + _default_prompt: ClassVar[str] =3D "testpmd>" > + > + #: This forces the prompt to appear after sending a command. > + _command_extra_chars: ClassVar[str] =3D "\n" > > def _start_application(self, get_privileged_command: Callable[[str], > str] | None) -> None: > - """See "_start_application" in InteractiveShell.""" > self._app_args +=3D " -- -i" > super()._start_application(get_privileged_command) > > def get_devices(self) -> list[TestPmdDevice]: > - """Get a list of device names that are known to testpmd > + """Get a list of device names that are known to testpmd. > > - Uses the device info listed in testpmd and then parses the outpu= t > to > - return only the names of the devices. > + Uses the device info listed in testpmd and then parses the outpu= t. > > Returns: > - A list of strings representing device names (e.g. > 0000:14:00.1) > + A list of devices. > """ > dev_info: str =3D self.send_command("show device info all") > dev_list: list[TestPmdDevice] =3D [] > -- > 2.34.1 > > --0000000000006e60f2060b65a2be Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


<= div dir=3D"ltr" class=3D"gmail_attr">On Thu, Nov 23, 2023 at 10:14=E2=80=AF= AM Juraj Linke=C5=A1 <juraj.linkes@pantheon.tech> wrote:
Format according to the Goog= le format and PEP257, with slight
deviations.

Signed-off-by: Juraj Linke=C5=A1 <juraj.linkes@pantheon.tech>
---
=C2=A0.../interactive_remote_session.py=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0| 36 +++----
=C2=A0.../remote_session/interactive_shell.py=C2=A0 =C2=A0 =C2=A0 =C2=A0| 9= 9 +++++++++++--------
=C2=A0dts/framework/remote_session/python_shell.py=C2=A0 | 26 ++++-
=C2=A0dts/framework/remote_session/testpmd_shell.py | 58 +++++++++--
=C2=A04 files changed, 149 insertions(+), 70 deletions(-)

diff --git a/dts/framework/remote_session/interactive_remote_session.py b/d= ts/framework/remote_session/interactive_remote_session.py
index 098ded1bb0..1cc82e3377 100644
--- a/dts/framework/remote_session/interactive_remote_session.py
+++ b/dts/framework/remote_session/interactive_remote_session.py
@@ -22,27 +22,23 @@
=C2=A0class InteractiveRemoteSession:
=C2=A0 =C2=A0 =C2=A0"""SSH connection dedicated to interacti= ve applications.

-=C2=A0 =C2=A0 This connection is created using paramiko and is a persisten= t connection to the
-=C2=A0 =C2=A0 host. This class defines methods for connecting to the node = and configures this
-=C2=A0 =C2=A0 connection to send "keep alive" packets every 30 s= econds. Because paramiko attempts
-=C2=A0 =C2=A0 to use SSH keys to establish a connection first, providing a= password is optional.
-=C2=A0 =C2=A0 This session is utilized by InteractiveShells and cannot be = interacted with
-=C2=A0 =C2=A0 directly.
-
-=C2=A0 =C2=A0 Arguments:
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 node_config: Configuration class for the node = you are connecting to.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 _logger: Desired logger for this session to us= e.
+=C2=A0 =C2=A0 The connection is created using `paramiko <htt= ps://docs.paramiko.org/en/latest/>`_
+=C2=A0 =C2=A0 and is a persistent connection to the host. This class defin= es the methods for connecting
+=C2=A0 =C2=A0 to the node and configures the connection to send "keep= alive" packets every 30 seconds.
+=C2=A0 =C2=A0 Because paramiko attempts to use SSH keys to establish a con= nection first, providing
+=C2=A0 =C2=A0 a password is optional. This session is utilized by Interact= iveShells
+=C2=A0 =C2=A0 and cannot be interacted with directly.

=C2=A0 =C2=A0 =C2=A0Attributes:
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 hostname: Hostname that will be used to initia= lize a connection to the node.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 ip: A subsection of hostname that removes the = port for the connection if there
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 hostname: The hostname that will be used to in= itialize a connection to the node.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 ip: A subsection of `hostname` that removes th= e port for the connection if there
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0is one. If there is no port= , this will be the same as hostname.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 port: Port to use for the ssh connection. This= will be extracted from the
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 hostname if there is a port incl= uded, otherwise it will default to 22.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 port: Port to use for the ssh connection. This= will be extracted from `hostname`
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if there is a port included, oth= erwise it will default to ``22``.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0username: User to connect to the node wit= h.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0password: Password of the user connecting= to the host. This will default to an
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0empty string if a password = is not provided.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 session: Underlying paramiko connection.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 session: The underlying paramiko connection.
=C2=A0 =C2=A0 =C2=A0Raises:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0SSHConnectionError: There is an error cre= ating the SSH connection.
@@ -58,9 +54,15 @@ class InteractiveRemoteSession:
=C2=A0 =C2=A0 =C2=A0_node_config: NodeConfiguration
=C2=A0 =C2=A0 =C2=A0_transport: Transport | None

-=C2=A0 =C2=A0 def __init__(self, node_config: NodeConfiguration, _logger: = DTSLOG) -> None:
+=C2=A0 =C2=A0 def __init__(self, node_config: NodeConfiguration, logger: D= TSLOG) -> None:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Connect to the node during i= nitialization.
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 Args:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 node_config: The test run config= uration of the node to connect to.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 logger: The logger instance this= session will use.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self._node_config =3D node_config
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._logger =3D _logger
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 self._logger =3D logger
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self.hostname =3D node_config.hostname =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self.username =3D node_config.user
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self.password =3D node_config.password if= node_config.password else ""
diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framew= ork/remote_session/interactive_shell.py
index 4db19fb9b3..b158f963b6 100644
--- a/dts/framework/remote_session/interactive_shell.py
+++ b/dts/framework/remote_session/interactive_shell.py
@@ -3,18 +3,20 @@

=C2=A0"""Common functionality for interactive shell handling= .

-This base class, InteractiveShell, is meant to be extended by other classe= s that
-contain functionality specific to that shell type. These derived classes w= ill often
-modify things like the prompt to expect or the arguments to pass into the = application,
-but still utilize the same method for sending a command and collecting out= put. How
-this output is handled however is often application specific. If an applic= ation needs
-elevated privileges to start it is expected that the method for gaining th= ose
-privileges is provided when initializing the class.
+The base class, :class:`InteractiveShell`, is meant to be extended by subc= lasses that contain
+functionality specific to that shell type. These subclasses will often mod= ify things like
+the prompt to expect or the arguments to pass into the application, but st= ill utilize
+the same method for sending a command and collecting output. How this outp= ut is handled however
+is often application specific. If an application needs elevated privileges= to start it is expected
+that the method for gaining those privileges is provided when initializing= the class.
+
+The :option:`--timeout` command line argument and the :envvar:`DTS_TIMEOUT= `
+environment variable configure the timeout of getting the output from comm= and execution.
=C2=A0"""

=C2=A0from abc import ABC
=C2=A0from pathlib import PurePath
-from typing import Callable
+from typing import Callable, ClassVar

=C2=A0from paramiko import Channel, SSHClient, channel=C2=A0 # type: ignore= [import]

@@ -30,28 +32,6 @@ class InteractiveShell(ABC):
=C2=A0 =C2=A0 =C2=A0and collecting input until reaching a certain prompt. A= ll interactive applications
=C2=A0 =C2=A0 =C2=A0will use the same SSH connection, but each will create = their own channel on that
=C2=A0 =C2=A0 =C2=A0session.
-
-=C2=A0 =C2=A0 Arguments:
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 interactive_session: The SSH session dedicated= to interactive shells.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 logger: Logger used for displaying information= in the console.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 get_privileged_command: Method for modifying a= command to allow it to use
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 elevated privileges. If this is = None, the application will not be started
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 with elevated privileges.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 app_args: Command line arguments to be passed = to the application on startup.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 timeout: Timeout used for the SSH channel that= is dedicated to this interactive
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 shell. This timeout is for colle= cting output, so if reading from the buffer
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 and no output is gathered within= the timeout, an exception is thrown.
-
-=C2=A0 =C2=A0 Attributes
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 _default_prompt: Prompt to expect at the end o= f output when sending a command.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 This is often overridden by deri= ved classes.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 _command_extra_chars: Extra characters to add = to the end of every command
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 before sending them. This is oft= en overridden by derived classes and is
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 most commonly an additional newl= ine character.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 path: Path to the executable to start the inte= ractive application.
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 dpdk_app: Whether this application is a DPDK a= pp. If it is, the build
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 directory for DPDK on the node w= ill be prepended to the path to the
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 executable.
=C2=A0 =C2=A0 =C2=A0"""

=C2=A0 =C2=A0 =C2=A0_interactive_session: SSHClient
@@ -61,10 +41,22 @@ class InteractiveShell(ABC):
=C2=A0 =C2=A0 =C2=A0_logger: DTSLOG
=C2=A0 =C2=A0 =C2=A0_timeout: float
=C2=A0 =C2=A0 =C2=A0_app_args: str
-=C2=A0 =C2=A0 _default_prompt: str =3D ""
-=C2=A0 =C2=A0 _command_extra_chars: str =3D ""
-=C2=A0 =C2=A0 path: PurePath
-=C2=A0 =C2=A0 dpdk_app: bool =3D False
+
+=C2=A0 =C2=A0 #: Prompt to expect at the end of output when sending a comm= and.
+=C2=A0 =C2=A0 #: This is often overridden by subclasses.
+=C2=A0 =C2=A0 _default_prompt: ClassVar[str] =3D ""
+
+=C2=A0 =C2=A0 #: Extra characters to add to the end of every command
+=C2=A0 =C2=A0 #: before sending them. This is often overridden by subclass= es and is
+=C2=A0 =C2=A0 #: most commonly an additional newline character.
+=C2=A0 =C2=A0 _command_extra_chars: ClassVar[str] =3D ""
+
+=C2=A0 =C2=A0 #: Path to the executable to start the interactive applicati= on.
+=C2=A0 =C2=A0 path: ClassVar[PurePath]
+
+=C2=A0 =C2=A0 #: Whether this application is a DPDK app. If it is, the bui= ld directory
+=C2=A0 =C2=A0 #: for DPDK on the node will be prepended to the path to the= executable.
+=C2=A0 =C2=A0 dpdk_app: ClassVar[bool] =3D False

=C2=A0 =C2=A0 =C2=A0def __init__(
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self,
@@ -74,6 +66,19 @@ def __init__(
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0app_args: str =3D "",
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0timeout: float =3D SETTINGS.timeout,
=C2=A0 =C2=A0 =C2=A0) -> None:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Create an SSH channel during= initialization.
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 Args:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 interactive_session: The SSH ses= sion dedicated to interactive shells.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 logger: The logger instance this= session will use.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 get_privileged_command: A method= for modifying a command to allow it to use
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 elevated privilege= s. If :data:`None`, the application will not be started
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 with elevated priv= ileges.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 app_args: The command line argum= ents to be passed to the application on startup.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 timeout: The timeout used for th= e SSH channel that is dedicated to this interactive
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 shell. This timeou= t is for collecting output, so if reading from the buffer
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 and no output is g= athered within the timeout, an exception is thrown.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self._interactive_session =3D interactive= _session
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self._ssh_channel =3D self._interactive_s= ession.invoke_shell()
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self._stdin =3D self._ssh_channel.makefil= e_stdin("w")
@@ -90,6 +95,10 @@ def _start_application(self, get_privileged_command: Cal= lable[[str], str] | None

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0This method is often overridden by subcla= sses as their process for
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0starting may look different.
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 Args:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 get_privileged_command: A functi= on (but could be any callable) that produces
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 the version of the= command with elevated privileges.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"""
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0start_command =3D f"{self.path} {sel= f._app_args}"
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if get_privileged_command is not None: @@ -97,16 +106,24 @@ def _start_application(self, get_privileged_command: C= allable[[str], str] | None
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self.send_command(start_command)

=C2=A0 =C2=A0 =C2=A0def send_command(self, command: str, prompt: str | None= =3D None) -> str:
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Send a command and get all o= utput before the expected ending string.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Send `command` and get all o= utput before the expected ending string.

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Lines that expect input are not included = in the stdout buffer, so they cannot
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 be used for expect. For example, if you were p= rompted to log into something
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 with a username and password, you cannot expec= t "username:" because it won't
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 yet be in the stdout buffer. A workaround for = this could be consuming an
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 extra newline character to force the current p= rompt into the stdout buffer.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 be used for expect.
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 Example:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 If you were prompted to log into= something with a username and password,
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 you cannot expect ``username:`` = because it won't yet be in the stdout buffer.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 A workaround for this could be c= onsuming an extra newline character to force
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 the current `prompt` into the st= dout buffer.
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 Args:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 command: The command to send. +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 prompt: After sending the comman= d, `send_command` will be expecting this string.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 If :data:`None`, w= ill use the class's default prompt.

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Returns:
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 All output in the buffer before = expected string
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 All output in the buffer before = expected string.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"""
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self._logger.info(f"Sending: '{com= mand}'")
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if prompt is None:
@@ -124,8 +141,10 @@ def send_command(self, command: str, prompt: str | Non= e =3D None) -> str:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return out

=C2=A0 =C2=A0 =C2=A0def close(self) -> None:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Properly free all resources.= """
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self._stdin.close()
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self._ssh_channel.close()

=C2=A0 =C2=A0 =C2=A0def __del__(self) -> None:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Make sure the session is pro= perly closed before deleting the object."""
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self.close()
diff --git a/dts/framework/remote_session/python_shell.py b/dts/framework/r= emote_session/python_shell.py
index cc3ad48a68..ccfd3783e8 100644
--- a/dts/framework/remote_session/python_shell.py
+++ b/dts/framework/remote_session/python_shell.py
@@ -1,12 +1,32 @@
=C2=A0# SPDX-License-Identifier: BSD-3-Clause
=C2=A0# Copyright(c) 2023 PANTHEON.tech s.r.o.

+"""Python interactive shell.
+
+Typical usage example in a TestSuite::
+
+=C2=A0 =C2=A0 from framework.remote_session import PythonShell
+=C2=A0 =C2=A0 python_shell =3D self.tg_node.create_interactive_shell(
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 PythonShell, timeout=3D5, privileged=3DTrue +=C2=A0 =C2=A0 )
+=C2=A0 =C2=A0 python_shell.send_command("print('Hello World')= ")
+=C2=A0 =C2=A0 python_shell.close()
+"""
+
=C2=A0from pathlib import PurePath
+from typing import ClassVar

=C2=A0from .interactive_shell import InteractiveShell


=C2=A0class PythonShell(InteractiveShell):
-=C2=A0 =C2=A0 _default_prompt: str =3D ">>>"
-=C2=A0 =C2=A0 _command_extra_chars: str =3D "\n"
-=C2=A0 =C2=A0 path: PurePath =3D PurePath("python3")
+=C2=A0 =C2=A0 """Python interactive shell."""= ;
+
+=C2=A0 =C2=A0 #: Python's prompt.
+=C2=A0 =C2=A0 _default_prompt: ClassVar[str] =3D ">>>"<= br> +
+=C2=A0 =C2=A0 #: This forces the prompt to appear after sending a command.=
+=C2=A0 =C2=A0 _command_extra_chars: ClassVar[str] =3D "\n"
+
+=C2=A0 =C2=A0 #: The Python executable.
+=C2=A0 =C2=A0 path: ClassVar[PurePath] =3D PurePath("python3") diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/= remote_session/testpmd_shell.py
index 08ac311016..79481e845c 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -1,41 +1,79 @@
=C2=A0# SPDX-License-Identifier: BSD-3-Clause
=C2=A0# Copyright(c) 2023 University of New Hampshire


Should you add to the copyright here for addi= ng comments?
=C2=A0
+"""Testpmd interactive shell.
+
+Typical usage example in a TestSuite::
+
+=C2=A0 =C2=A0 testpmd_shell =3D self.sut_node.create_interactive_shell( +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 TestPmdShell, privileged=3DTrue<= br> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 )
+=C2=A0 =C2=A0 devices =3D testpmd_shell.get_devices()
+=C2=A0 =C2=A0 for device in devices:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 print(device)
+=C2=A0 =C2=A0 testpmd_shell.close()
+"""
+
=C2=A0from pathlib import PurePath
-from typing import Callable
+from typing import Callable, ClassVar

=C2=A0from .interactive_shell import InteractiveShell


=C2=A0class TestPmdDevice(object):
+=C2=A0 =C2=A0 """The data of a device that testpmd can reco= gnize.
+
+=C2=A0 =C2=A0 Attributes:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 pci_address: The PCI address of the device. +=C2=A0 =C2=A0 """
+
=C2=A0 =C2=A0 =C2=A0pci_address: str

=C2=A0 =C2=A0 =C2=A0def __init__(self, pci_address_line: str):
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Initialize the device from t= he testpmd output line string.
+
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 Args:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 pci_address_line: A line of test= pmd output that contains a device.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self.pci_address =3D pci_address_line.str= ip().split(": ")[1].strip()

=C2=A0 =C2=A0 =C2=A0def __str__(self) -> str:
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """The PCI address captures wha= t the device is."""
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return self.pci_address


=C2=A0class TestPmdShell(InteractiveShell):
-=C2=A0 =C2=A0 path: PurePath =3D PurePath("app", "dpdk-test= pmd")
-=C2=A0 =C2=A0 dpdk_app: bool =3D True
-=C2=A0 =C2=A0 _default_prompt: str =3D "testpmd>"
-=C2=A0 =C2=A0 _command_extra_chars: str =3D "\n"=C2=A0 # We want= to append an extra newline to every command
+=C2=A0 =C2=A0 """Testpmd interactive shell.
+
+=C2=A0 =C2=A0 The testpmd shell users should never use
+=C2=A0 =C2=A0 the :meth:`~.interactive_shell.InteractiveShell.send_command= ` method directly, but rather
+=C2=A0 =C2=A0 call specialized methods. If there isn't one that satisf= ies a need, it should be added.
+=C2=A0 =C2=A0 """
+
+=C2=A0 =C2=A0 #: The path to the testpmd executable.
+=C2=A0 =C2=A0 path: ClassVar[PurePath] =3D PurePath("app", "= ;dpdk-testpmd")
+
+=C2=A0 =C2=A0 #: Flag this as a DPDK app so that it's clear this is no= t a system app and
+=C2=A0 =C2=A0 #: needs to be looked in a specific path.
+=C2=A0 =C2=A0 dpdk_app: ClassVar[bool] =3D True
+
+=C2=A0 =C2=A0 #: The testpmd's prompt.
+=C2=A0 =C2=A0 _default_prompt: ClassVar[str] =3D "testpmd>" +
+=C2=A0 =C2=A0 #: This forces the prompt to appear after sending a command.=
+=C2=A0 =C2=A0 _command_extra_chars: ClassVar[str] =3D "\n"

=C2=A0 =C2=A0 =C2=A0def _start_application(self, get_privileged_command: Ca= llable[[str], str] | None) -> None:
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 """See "_start_application= " in InteractiveShell."""
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0self._app_args +=3D " -- -i" =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0super()._start_application(get_privileged= _command)

=C2=A0 =C2=A0 =C2=A0def get_devices(self) -> list[TestPmdDevice]:
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Get a list of device names t= hat are known to testpmd
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 """Get a list of device names t= hat are known to testpmd.

-=C2=A0 =C2=A0 =C2=A0 =C2=A0 Uses the device info listed in testpmd and the= n parses the output to
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 return only the names of the devices.
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 Uses the device info listed in testpmd and the= n parses the output.

=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Returns:
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 A list of strings representing d= evice names (e.g. 0000:14:00.1)
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 A list of devices.
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0"""
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_info: str =3D self.send_command("= ;show device info all")
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0dev_list: list[TestPmdDevice] =3D []
--
2.34.1

--0000000000006e60f2060b65a2be--