[PATCH v5 2/8] dts: use Params for interactive shells

[Luca Vizzarro <luca.vizzarro@arm.com>]

Make it so that interactive shells accept an implementation of `Params`
for app arguments. Convert EalParameters to use `Params` instead.

String command line parameters can still be supplied by using the
`Params.from_str()` method.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Paul Szczepanek <paul.szczepanek@arm.com>
Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
Reviewed-by: Nicholas Pratte <npratte@iol.unh.edu>
---
 .../remote_session/interactive_shell.py       |  12 +-
 dts/framework/remote_session/testpmd_shell.py |  11 +-
 dts/framework/testbed_model/node.py           |   6 +-
 dts/framework/testbed_model/os_session.py     |   4 +-
 dts/framework/testbed_model/sut_node.py       | 124 ++++++++----------
 dts/tests/TestSuite_pmd_buffer_scatter.py     |   3 +-
 6 files changed, 77 insertions(+), 83 deletions(-)

diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
index c025c52ba3..8191b36630 100644
--- a/dts/framework/remote_session/interactive_shell.py
+++ b/dts/framework/remote_session/interactive_shell.py
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2023 University of New Hampshire
+# Copyright(c) 2024 Arm Limited
 
 """Common functionality for interactive shell handling.
 
@@ -21,6 +22,7 @@
 from paramiko import Channel, SSHClient, channel  # type: ignore[import-untyped]
 
 from framework.logger import DTSLogger
+from framework.params import Params
 from framework.settings import SETTINGS
 
 
@@ -40,7 +42,7 @@ class InteractiveShell(ABC):
     _ssh_channel: Channel
     _logger: DTSLogger
     _timeout: float
-    _app_args: str
+    _app_params: Params
 
     #: Prompt to expect at the end of output when sending a command.
     #: This is often overridden by subclasses.
@@ -63,7 +65,7 @@ def __init__(
         interactive_session: SSHClient,
         logger: DTSLogger,
         get_privileged_command: Callable[[str], str] | None,
-        app_args: str = "",
+        app_params: Params = Params(),
         timeout: float = SETTINGS.timeout,
     ) -> None:
         """Create an SSH channel during initialization.
@@ -74,7 +76,7 @@ def __init__(
             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.
+            app_params: The command line parameters 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.
@@ -87,7 +89,7 @@ def __init__(
         self._ssh_channel.set_combine_stderr(True)  # combines stdout and stderr streams
         self._logger = logger
         self._timeout = timeout
-        self._app_args = app_args
+        self._app_params = app_params
         self._start_application(get_privileged_command)
 
     def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None:
@@ -100,7 +102,7 @@ def _start_application(self, get_privileged_command: Callable[[str], str] | None
             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}"
+        start_command = f"{self.path} {self._app_params}"
         if get_privileged_command is not None:
             start_command = get_privileged_command(start_command)
         self.send_command(start_command)
diff --git a/dts/framework/remote_session/testpmd_shell.py b/dts/framework/remote_session/testpmd_shell.py
index d413bf2cc7..2836ed5c48 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -28,6 +28,7 @@
 from framework.exception import InteractiveCommandExecutionError
 from framework.parser import ParserFn, TextParser
 from framework.settings import SETTINGS
+from framework.testbed_model.sut_node import EalParams
 from framework.utils import StrEnum
 
 from .interactive_shell import InteractiveShell
@@ -645,8 +646,14 @@ def _start_application(self, get_privileged_command: Callable[[str], str] | None
         Also find the number of pci addresses which were allowed on the command line when the app
         was started.
         """
-        self._app_args += " -i --mask-event intr_lsc"
-        self.number_of_ports = self._app_args.count("-a ")
+        self._app_params += " -i --mask-event intr_lsc"
+
+        assert isinstance(self._app_params, EalParams)
+
+        self.number_of_ports = (
+            len(self._app_params.ports) if self._app_params.ports is not None else 0
+        )
+
         super()._start_application(get_privileged_command)
 
     def start(self, verify: bool = True) -> None:
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 74061f6262..6af4f25a3c 100644
--- a/dts/framework/testbed_model/node.py
+++ b/dts/framework/testbed_model/node.py
@@ -2,6 +2,7 @@
 # Copyright(c) 2010-2014 Intel Corporation
 # Copyright(c) 2022-2023 PANTHEON.tech s.r.o.
 # Copyright(c) 2022-2023 University of New Hampshire
+# Copyright(c) 2024 Arm Limited
 
 """Common functionality for node management.
 
@@ -24,6 +25,7 @@
 )
 from framework.exception import ConfigurationError
 from framework.logger import DTSLogger, get_dts_logger
+from framework.params import Params
 from framework.settings import SETTINGS
 
 from .cpu import (
@@ -199,7 +201,7 @@ def create_interactive_shell(
         shell_cls: Type[InteractiveShellType],
         timeout: float = SETTINGS.timeout,
         privileged: bool = False,
-        app_args: str = "",
+        app_params: Params = Params(),
     ) -> InteractiveShellType:
         """Factory for interactive session handlers.
 
@@ -222,7 +224,7 @@ def create_interactive_shell(
             shell_cls,
             timeout,
             privileged,
-            app_args,
+            app_params,
         )
 
     def filter_lcores(
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index d5bf7e0401..1a77aee532 100644
--- a/dts/framework/testbed_model/os_session.py
+++ b/dts/framework/testbed_model/os_session.py
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2023 PANTHEON.tech s.r.o.
 # Copyright(c) 2023 University of New Hampshire
+# Copyright(c) 2024 Arm Limited
 
 """OS-aware remote session.
 
@@ -29,6 +30,7 @@
 
 from framework.config import Architecture, NodeConfiguration, NodeInfo
 from framework.logger import DTSLogger
+from framework.params import Params
 from framework.remote_session import (
     CommandResult,
     InteractiveRemoteSession,
@@ -134,7 +136,7 @@ def create_interactive_shell(
         shell_cls: Type[InteractiveShellType],
         timeout: float,
         privileged: bool,
-        app_args: str,
+        app_args: Params,
     ) -> InteractiveShellType:
         """Factory for interactive session handlers.
 
diff --git a/dts/framework/testbed_model/sut_node.py b/dts/framework/testbed_model/sut_node.py
index 97aa26d419..c886590979 100644
--- a/dts/framework/testbed_model/sut_node.py
+++ b/dts/framework/testbed_model/sut_node.py
@@ -2,6 +2,7 @@
 # Copyright(c) 2010-2014 Intel Corporation
 # Copyright(c) 2023 PANTHEON.tech s.r.o.
 # Copyright(c) 2023 University of New Hampshire
+# Copyright(c) 2024 Arm Limited
 
 """System under test (DPDK + hardware) node.
 
@@ -14,8 +15,9 @@
 import os
 import tarfile
 import time
+from dataclasses import dataclass, field
 from pathlib import PurePath
-from typing import Type
+from typing import Literal, Type
 
 from framework.config import (
     BuildTargetConfiguration,
@@ -23,6 +25,7 @@
     NodeInfo,
     SutNodeConfiguration,
 )
+from framework.params import Params, Switch
 from framework.remote_session import CommandResult
 from framework.settings import SETTINGS
 from framework.utils import MesonArgs
@@ -34,62 +37,42 @@
 from .virtual_device import VirtualDevice
 
 
-class EalParameters(object):
-    """The environment abstraction layer parameters.
-
-    The string representation can be created by converting the instance to a string.
-    """
+def _port_to_pci(port: Port) -> str:
+    return port.pci
 
-    def __init__(
-        self,
-        lcore_list: LogicalCoreList,
-        memory_channels: int,
-        prefix: str,
-        no_pci: bool,
-        vdevs: list[VirtualDevice],
-        ports: list[Port],
-        other_eal_param: str,
-    ):
-        """Initialize the parameters according to inputs.
-
-        Process the parameters into the format used on the command line.
 
-        Args:
-            lcore_list: The list of logical cores to use.
-            memory_channels: The number of memory channels to use.
-            prefix: Set the file prefix string with which to start DPDK, e.g.: ``prefix='vf'``.
-            no_pci: Switch to disable PCI bus e.g.: ``no_pci=True``.
-            vdevs: Virtual devices, e.g.::
+@dataclass(kw_only=True)
+class EalParams(Params):
+    """The environment abstraction layer parameters.
 
-                vdevs=[
-                    VirtualDevice('net_ring0'),
-                    VirtualDevice('net_ring1')
-                ]
-            ports: The list of ports to allow.
-            other_eal_param: user defined DPDK EAL parameters, e.g.:
+    Attributes:
+        lcore_list: The list of logical cores to use.
+        memory_channels: The number of memory channels to use.
+        prefix: Set the file prefix string with which to start DPDK, e.g.: ``prefix="vf"``.
+        no_pci: Switch to disable PCI bus, e.g.: ``no_pci=True``.
+        vdevs: Virtual devices, e.g.::
+            vdevs=[
+                VirtualDevice('net_ring0'),
+                VirtualDevice('net_ring1')
+            ]
+        ports: The list of ports to allow.
+        other_eal_param: user defined DPDK EAL parameters, e.g.:
                 ``other_eal_param='--single-file-segments'``
-        """
-        self._lcore_list = f"-l {lcore_list}"
-        self._memory_channels = f"-n {memory_channels}"
-        self._prefix = prefix
-        if prefix:
-            self._prefix = f"--file-prefix={prefix}"
-        self._no_pci = "--no-pci" if no_pci else ""
-        self._vdevs = " ".join(f"--vdev {vdev}" for vdev in vdevs)
-        self._ports = " ".join(f"-a {port.pci}" for port in ports)
-        self._other_eal_param = other_eal_param
-
-    def __str__(self) -> str:
-        """Create the EAL string."""
-        return (
-            f"{self._lcore_list} "
-            f"{self._memory_channels} "
-            f"{self._prefix} "
-            f"{self._no_pci} "
-            f"{self._vdevs} "
-            f"{self._ports} "
-            f"{self._other_eal_param}"
-        )
+    """
+
+    lcore_list: LogicalCoreList = field(metadata=Params.short("l"))
+    memory_channels: int = field(metadata=Params.short("n"))
+    prefix: str = field(metadata=Params.long("file-prefix"))
+    no_pci: Switch
+    vdevs: list[VirtualDevice] | None = field(
+        default=None, metadata=Params.multiple() | Params.long("vdev")
+    )
+    ports: list[Port] | None = field(
+        default=None,
+        metadata=Params.convert_value(_port_to_pci) | Params.multiple() | Params.short("a"),
+    )
+    other_eal_param: Params | None = None
+    _separator: Literal[True] = field(default=True, init=False, metadata=Params.short("-"))
 
 
 class SutNode(Node):
@@ -350,11 +333,11 @@ def create_eal_parameters(
         ascending_cores: bool = True,
         prefix: str = "dpdk",
         append_prefix_timestamp: bool = True,
-        no_pci: bool = False,
+        no_pci: Switch = None,
         vdevs: list[VirtualDevice] | None = None,
         ports: list[Port] | None = None,
         other_eal_param: str = "",
-    ) -> "EalParameters":
+    ) -> EalParams:
         """Compose the EAL parameters.
 
         Process the list of cores and the DPDK prefix and pass that along with
@@ -393,24 +376,21 @@ def create_eal_parameters(
         if prefix:
             self._dpdk_prefix_list.append(prefix)
 
-        if vdevs is None:
-            vdevs = []
-
         if ports is None:
             ports = self.ports
 
-        return EalParameters(
+        return EalParams(
             lcore_list=lcore_list,
             memory_channels=self.config.memory_channels,
             prefix=prefix,
             no_pci=no_pci,
             vdevs=vdevs,
             ports=ports,
-            other_eal_param=other_eal_param,
+            other_eal_param=Params.from_str(other_eal_param),
         )
 
     def run_dpdk_app(
-        self, app_path: PurePath, eal_args: "EalParameters", timeout: float = 30
+        self, app_path: PurePath, eal_params: EalParams, timeout: float = 30
     ) -> CommandResult:
         """Run DPDK application on the remote node.
 
@@ -419,14 +399,14 @@ def run_dpdk_app(
 
         Args:
             app_path: The remote path to the DPDK application.
-            eal_args: EAL parameters to run the DPDK application with.
+            eal_params: EAL parameters to run the DPDK application with.
             timeout: Wait at most this long in seconds for `command` execution to complete.
 
         Returns:
             The result of the DPDK app execution.
         """
         return self.main_session.send_command(
-            f"{app_path} {eal_args}", timeout, privileged=True, verify=True
+            f"{app_path} {eal_params}", timeout, privileged=True, verify=True
         )
 
     def configure_ipv4_forwarding(self, enable: bool) -> None:
@@ -442,8 +422,8 @@ def create_interactive_shell(
         shell_cls: Type[InteractiveShellType],
         timeout: float = SETTINGS.timeout,
         privileged: bool = False,
-        app_parameters: str = "",
-        eal_parameters: EalParameters | None = None,
+        app_params: Params = Params(),
+        eal_params: EalParams | None = None,
     ) -> InteractiveShellType:
         """Extend the factory for interactive session handlers.
 
@@ -459,26 +439,26 @@ def create_interactive_shell(
                 reading from the buffer and don't receive any data within the timeout
                 it will throw an error.
             privileged: Whether to run the shell with administrative privileges.
-            eal_parameters: List of EAL parameters to use to launch the app. If this
+            app_params: The parameters to be passed to the application.
+            eal_params: List of EAL parameters to use to launch the app. If this
                 isn't provided or an empty string is passed, it will default to calling
                 :meth:`create_eal_parameters`.
-            app_parameters: Additional arguments to pass into the application on the
-                command-line.
 
         Returns:
             An instance of the desired interactive application shell.
         """
         # We need to append the build directory and add EAL parameters for DPDK apps
         if shell_cls.dpdk_app:
-            if not eal_parameters:
-                eal_parameters = self.create_eal_parameters()
-            app_parameters = f"{eal_parameters} -- {app_parameters}"
+            if eal_params is None:
+                eal_params = self.create_eal_parameters()
+            eal_params.append_str(str(app_params))
+            app_params = eal_params
 
             shell_cls.path = self.main_session.join_remote_path(
                 self.remote_dpdk_build_dir, shell_cls.path
             )
 
-        return super().create_interactive_shell(shell_cls, timeout, privileged, app_parameters)
+        return super().create_interactive_shell(shell_cls, timeout, privileged, app_params)
 
     def bind_ports_to_driver(self, for_dpdk: bool = True) -> None:
         """Bind all ports on the SUT to a driver.
diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_pmd_buffer_scatter.py
index a020682e8d..c6e93839cb 100644
--- a/dts/tests/TestSuite_pmd_buffer_scatter.py
+++ b/dts/tests/TestSuite_pmd_buffer_scatter.py
@@ -22,6 +22,7 @@
 from scapy.packet import Raw  # type: ignore[import-untyped]
 from scapy.utils import hexstr  # type: ignore[import-untyped]
 
+from framework.params import Params
 from framework.remote_session.testpmd_shell import TestPmdForwardingModes, TestPmdShell
 from framework.test_suite import TestSuite
 
@@ -103,7 +104,7 @@ def pmd_scatter(self, mbsize: int) -> None:
         """
         testpmd = self.sut_node.create_interactive_shell(
             TestPmdShell,
-            app_parameters=(
+            app_params=Params.from_str(
                 "--mbcache=200 "
                 f"--mbuf-size={mbsize} "
                 "--max-pkt-len=9000 "
-- 
2.34.1