From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id C5766432BB;
	Mon,  6 Nov 2023 18:17:49 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id AE63D427E5;
	Mon,  6 Nov 2023 18:16:24 +0100 (CET)
Received: from mail-ed1-f49.google.com (mail-ed1-f49.google.com
 [209.85.208.49]) by mails.dpdk.org (Postfix) with ESMTP id 1B63841149
 for <dev@dpdk.org>; Mon,  6 Nov 2023 18:16:22 +0100 (CET)
Received: by mail-ed1-f49.google.com with SMTP id
 4fb4d7f45d1cf-53dd3f169d8so7852470a12.3
 for <dev@dpdk.org>; Mon, 06 Nov 2023 09:16:22 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=pantheon.tech; s=google; t=1699290982; x=1699895782; darn=dpdk.org;
 h=content-transfer-encoding:mime-version:references:in-reply-to
 :message-id:date:subject:cc:to:from:from:to:cc:subject:date
 :message-id:reply-to;
 bh=DRBT1iCQ3izDtfSEhLlQAS7Wlh56tQtjJbqIRheTJP8=;
 b=h/1kOPsJedJ5gPbv0b+b4v2a2+aa5o1mUGzLsFZMdrC2VyMRtnHU0WZFo8t3B+zp1X
 kUd3HwMQerUxVSmCcoNN3pN2EIL0UZeruFrW3mFLVmJwUIaldf3cxB6vKrpH8eCCCy6C
 JQ9yUfHzPMniKWoJAESqMrJOObA4+DKzaYuzDbsu/8tCmxg1vFkz9z/mL1HbmfEj2v0h
 AOBiEOeQT13+okevFGgFB6anCqU9qPWGD3sj8dmEMayzQvK++MKG8yt6e1IztWSBYGyH
 NYn3pLOkC5nxaUT419A6vMPD2EII9XaYLT/1jHuHDVKB5jqmLOD9q8uXtxTN5FqCXeTo
 BihA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20230601; t=1699290982; x=1699895782;
 h=content-transfer-encoding:mime-version:references:in-reply-to
 :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc
 :subject:date:message-id:reply-to;
 bh=DRBT1iCQ3izDtfSEhLlQAS7Wlh56tQtjJbqIRheTJP8=;
 b=JNMNsQw30x+B5Vicikbng+RpeAr0JFmveNIQR+NJ000lmG6eF9D+VWyLqGbrvSzExR
 h5eQriNderBGhRuX5bDSusxyKZwE0A7IxJm0NYwXmwQjJWibfXMjV8TKUve756vIHc7f
 uyTt6afE0qYSJvjjZMv06mMDkreq53bBsr8+OUgYLdnZvNuJuVTWsznnK71p/8n6f5nw
 Z2W2z2Z/qWtKICxVy/mNS91LUfX9uiWFkBqig8Zzyg1YIdS+gspaEsW8zIViUY6dOqUf
 Bd+QH1jPIgUX5mdwb663wwn9y+x3AwxYfRAt5bZj7+tuIsFy4ddM8RjD1BJYiyNgJWAe
 4ZNw==
X-Gm-Message-State: AOJu0YyM6kGmVJ7UVCF5DOXLrWDWFH0GodyDQBq1IJ8Cix+GeKaDvMPO
 ymwfEbOpAZd0oxzUc/Fs2JhwYA==
X-Google-Smtp-Source: AGHT+IEEC4leYqszpjM8yUwhZMMZfVJ0IliLpmErodDILXBYqlrvCWtR6pvXbMZD2SbCBgKcRuN96A==
X-Received: by 2002:a17:906:fe05:b0:9d2:5cf8:e61 with SMTP id
 wy5-20020a170906fe0500b009d25cf80e61mr14170321ejb.35.1699290981695; 
 Mon, 06 Nov 2023 09:16:21 -0800 (PST)
Received: from jlinkes-PT-Latitude-5530.. (ip-46.34.243.197.o2inet.sk.
 [46.34.243.197]) by smtp.gmail.com with ESMTPSA id
 s10-20020a170906354a00b009b947aacb4bsm47016eja.191.2023.11.06.09.16.20
 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
 Mon, 06 Nov 2023 09:16:21 -0800 (PST)
From: =?UTF-8?q?Juraj=20Linke=C5=A1?= <juraj.linkes@pantheon.tech>
To: thomas@monjalon.net, Honnappa.Nagarahalli@arm.com,
 bruce.richardson@intel.com, jspewock@iol.unh.edu, probb@iol.unh.edu,
 paul.szczepanek@arm.com, yoan.picchi@foss.arm.com
Cc: dev@dpdk.org, =?UTF-8?q?Juraj=20Linke=C5=A1?= <juraj.linkes@pantheon.tech>
Subject: [PATCH v5 12/23] dts: interactive remote session docstring update
Date: Mon,  6 Nov 2023 18:15:50 +0100
Message-Id: <20231106171601.160749-13-juraj.linkes@pantheon.tech>
X-Mailer: git-send-email 2.34.1
In-Reply-To: <20231106171601.160749-1-juraj.linkes@pantheon.tech>
References: <20230831100407.59865-1-juraj.linkes@pantheon.tech>
 <20231106171601.160749-1-juraj.linkes@pantheon.tech>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

Format according to the Google format and PEP257, with slight
deviations.

Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
 .../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 | 61 +++++++++---
 4 files changed, 150 insertions(+), 72 deletions(-)

diff --git a/dts/framework/remote_session/interactive_remote_session.py b/dts/framework/remote_session/interactive_remote_session.py
index 9085a668e8..c1bf30ac61 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 password 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 InteractiveShells
+    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 connection.
@@ -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 = node_config
-        self._logger = _logger
+        self._logger = logger
         self.hostname = node_config.hostname
         self.username = node_config.user
         self.password = node_config.password if node_config.password else ""
diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
index c24376b2a8..a98a822e91 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 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 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 interactive 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 is 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 every 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 application.
-        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 = ""
-    _command_extra_chars: str = ""
-    path: PurePath
-    dpdk_app: bool = False
+
+    #: Prompt to expect at the end of output when sending a command.
+    #: This is often overridden by subclasses.
+    _default_prompt: ClassVar[str] = ""
+
+    #: Extra characters to add to the end of every command
+    #: before sending them. This is often overridden by subclasses and is
+    #: most commonly an additional newline character.
+    _command_extra_chars: ClassVar[str] = ""
+
+    #: 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] = False
 
     def __init__(
         self,
@@ -74,6 +66,19 @@ def __init__(
         app_args: str = "",
         timeout: float = SETTINGS.timeout,
     ) -> None:
+        """Create an SSH channel during initialization.
+
+        Args:
+            interactive_session: The SSH session dedicated to interactive 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 = interactive_session
         self._ssh_channel = self._interactive_session.invoke_shell()
         self._stdin = self._ssh_channel.makefile_stdin("w")
@@ -92,6 +97,10 @@ def _start_application(
 
         This method is often overridden by subclasses as their process for
         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 = f"{self.path} {self._app_args}"
         if get_privileged_command is not None:
@@ -99,16 +108,24 @@ def _start_application(
         self.send_command(start_command)
 
     def send_command(self, command: str, prompt: str | None = 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, so they cannot
-        be used for expect. For 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.
+        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:
@@ -126,8 +143,10 @@ def send_command(self, command: str, prompt: str | None = 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..c8e5957ef7 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 = self.tg_node.create_interactive_shell(
+        PythonShell, timeout=5, privileged=True
+    )
+    python_shell.send_command("print('Hello World')")
+    pytyon_shell.close()
+"""
+
 from pathlib import PurePath
+from typing import ClassVar
 
 from .interactive_shell import InteractiveShell
 
 
 class PythonShell(InteractiveShell):
-    _default_prompt: str = ">>>"
-    _command_extra_chars: str = "\n"
-    path: PurePath = PurePath("python3")
+    """Python interactive shell."""
+
+    #: Python's prompt.
+    _default_prompt: ClassVar[str] = ">>>"
+
+    #: This forces the prompt to appear after sending a command.
+    _command_extra_chars: ClassVar[str] = "\n"
+
+    #: The Python executable.
+    path: ClassVar[PurePath] = PurePath("python3")
diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index 1455b5a199..2632515d74 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -1,45 +1,82 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2023 University of New Hampshire
 
+"""Testpmd interactive shell.
+
+Typical usage example in a TestSuite::
+
+    testpmd_shell = self.sut_node.create_interactive_shell(
+            TestPmdShell, privileged=True
+        )
+    devices = 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 = pci_address_line.strip().split(": ")[1].strip()
 
     def __str__(self) -> str:
+        """The PCI address captures what the device is."""
         return self.pci_address
 
 
 class TestPmdShell(InteractiveShell):
-    path: PurePath = PurePath("app", "dpdk-testpmd")
-    dpdk_app: bool = True
-    _default_prompt: str = "testpmd>"
-    _command_extra_chars: str = (
-        "\n"  # We want to append an extra newline to every command
-    )
+    """Testpmd interactive shell.
+
+    The testpmd shell users should never use
+    the :meth:`~framework.remote_session.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] = 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] = True
+
+    #: The testpmd's prompt.
+    _default_prompt: ClassVar[str] = "testpmd>"
+
+    #: This forces the prompt to appear after sending a command.
+    _command_extra_chars: ClassVar[str] = "\n"
 
     def _start_application(
         self, get_privileged_command: Callable[[str], str] | None
     ) -> None:
-        """See "_start_application" in InteractiveShell."""
         self._app_args += " -- -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 output to
-        return only the names of the devices.
+        Uses the device info listed in testpmd and then parses the output.
 
         Returns:
-            A list of strings representing device names (e.g. 0000:14:00.1)
+            A list of devices.
         """
         dev_info: str = self.send_command("show device info all")
         dev_list: list[TestPmdDevice] = []
-- 
2.34.1