[PATCH 2/6] 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
`StrParams` implementation.

Signed-off-by: Luca Vizzarro <luca.vizzarro@arm.com>
Reviewed-by: Jack Bond-Preston <jack.bond-preston@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
---
 .../remote_session/interactive_shell.py       |   8 +-
 dts/framework/remote_session/testpmd_shell.py |  12 +-
 dts/framework/testbed_model/__init__.py       |   2 +-
 dts/framework/testbed_model/node.py           |   4 +-
 dts/framework/testbed_model/os_session.py     |   4 +-
 dts/framework/testbed_model/sut_node.py       | 106 ++++++++----------
 dts/tests/TestSuite_pmd_buffer_scatter.py     |   3 +-
 7 files changed, 73 insertions(+), 66 deletions(-)

diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
index 5cfe202e15..a2c7b30d9f 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]
 
 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_args: Params | None
 
     #: 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_args: Params | None = None,
         timeout: float = SETTINGS.timeout,
     ) -> None:
         """Create an SSH channel during initialization.
@@ -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_args or ''}"
         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 cb2ab6bd00..db3abb7600 100644
--- a/dts/framework/remote_session/testpmd_shell.py
+++ b/dts/framework/remote_session/testpmd_shell.py
@@ -21,6 +21,7 @@
 from typing import Callable, ClassVar
 
 from framework.exception import InteractiveCommandExecutionError
+from framework.params import StrParams
 from framework.settings import SETTINGS
 from framework.utils import StrEnum
 
@@ -118,8 +119,15 @@ 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 ")
+        from framework.testbed_model.sut_node import EalParameters
+
+        assert isinstance(self._app_args, EalParameters)
+
+        if isinstance(self._app_args.app_params, StrParams):
+            self._app_args.app_params.value += " -i --mask-event intr_lsc"
+
+        self.number_of_ports = len(self._app_args.ports) if self._app_args.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/__init__.py b/dts/framework/testbed_model/__init__.py
index 6086512ca2..ef9520df4c 100644
--- a/dts/framework/testbed_model/__init__.py
+++ b/dts/framework/testbed_model/__init__.py
@@ -23,6 +23,6 @@
 from .cpu import LogicalCoreCount, LogicalCoreCountFilter, LogicalCoreList
 from .node import Node
 from .port import Port, PortLink
-from .sut_node import SutNode
+from .sut_node import SutNode, EalParameters
 from .tg_node import TGNode
 from .virtual_device import VirtualDevice
diff --git a/dts/framework/testbed_model/node.py b/dts/framework/testbed_model/node.py
index 74061f6262..ec9512d618 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_args: Params | None = None,
     ) -> InteractiveShellType:
         """Factory for interactive session handlers.
 
diff --git a/dts/framework/testbed_model/os_session.py b/dts/framework/testbed_model/os_session.py
index d5bf7e0401..7234c975c8 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 | None,
     ) -> 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..3f8c3807b3 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.
 
@@ -11,6 +12,7 @@
 """
 
 
+from dataclasses import dataclass, field
 import os
 import tarfile
 import time
@@ -23,6 +25,8 @@
     NodeInfo,
     SutNodeConfiguration,
 )
+from framework import params
+from framework.params import Params, StrParams
 from framework.remote_session import CommandResult
 from framework.settings import SETTINGS
 from framework.utils import MesonArgs
@@ -34,62 +38,51 @@
 from .virtual_device import VirtualDevice
 
 
-class EalParameters(object):
+def _port_to_pci(port: Port) -> str:
+    return port.pci
+
+
+@dataclass(kw_only=True)
+class EalParameters(Params):
     """The environment abstraction layer parameters.
 
     The string representation can be created by converting the instance to a string.
     """
 
-    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.
+    lcore_list: LogicalCoreList = field(metadata=params.short("l"))
+    """The list of logical cores to use."""
 
-        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.::
+    memory_channels: int = field(metadata=params.short("n"))
+    """The number of memory channels to use."""
 
-                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}"
-        )
+    prefix: str = field(metadata=params.long("file-prefix"))
+    """Set the file prefix string with which to start DPDK, e.g.: ``prefix="vf"``."""
+
+    no_pci: params.Option
+    """Switch to disable PCI bus e.g.: ``no_pci=True``."""
+
+    vdevs: list[VirtualDevice] | None = field(
+        default=None, metadata=params.multiple(params.long("vdev"))
+    )
+    """Virtual devices, e.g.::
+
+        vdevs=[
+            VirtualDevice("net_ring0"),
+            VirtualDevice("net_ring1")
+        ]
+    """
+
+    ports: list[Port] | None = field(
+        default=None,
+        metadata=params.field_mixins(_port_to_pci, metadata=params.multiple(params.short("a"))),
+    )
+    """The list of ports to allow."""
+
+    other_eal_param: StrParams | None = None
+    """Any other EAL parameter(s)."""
+
+    app_params: Params | None = field(default=None, metadata=params.options_end())
+    """Parameters to pass to the underlying DPDK app."""
 
 
 class SutNode(Node):
@@ -350,7 +343,7 @@ def create_eal_parameters(
         ascending_cores: bool = True,
         prefix: str = "dpdk",
         append_prefix_timestamp: bool = True,
-        no_pci: bool = False,
+        no_pci: params.Option = None,
         vdevs: list[VirtualDevice] | None = None,
         ports: list[Port] | None = None,
         other_eal_param: str = "",
@@ -393,9 +386,6 @@ def create_eal_parameters(
         if prefix:
             self._dpdk_prefix_list.append(prefix)
 
-        if vdevs is None:
-            vdevs = []
-
         if ports is None:
             ports = self.ports
 
@@ -406,7 +396,7 @@ def create_eal_parameters(
             no_pci=no_pci,
             vdevs=vdevs,
             ports=ports,
-            other_eal_param=other_eal_param,
+            other_eal_param=StrParams(other_eal_param),
         )
 
     def run_dpdk_app(
@@ -442,7 +432,7 @@ def create_interactive_shell(
         shell_cls: Type[InteractiveShellType],
         timeout: float = SETTINGS.timeout,
         privileged: bool = False,
-        app_parameters: str = "",
+        app_parameters: Params | None = None,
         eal_parameters: EalParameters | None = None,
     ) -> InteractiveShellType:
         """Extend the factory for interactive session handlers.
@@ -459,6 +449,7 @@ 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.
+            app_args: The arguments to be passed to the application.
             eal_parameters: 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`.
@@ -470,9 +461,10 @@ def create_interactive_shell(
         """
         # We need to append the build directory and add EAL parameters for DPDK apps
         if shell_cls.dpdk_app:
-            if not eal_parameters:
+            if eal_parameters is None:
                 eal_parameters = self.create_eal_parameters()
-            app_parameters = f"{eal_parameters} -- {app_parameters}"
+            eal_parameters.app_params = app_parameters
+            app_parameters = eal_parameters
 
             shell_cls.path = self.main_session.join_remote_path(
                 self.remote_dpdk_build_dir, shell_cls.path
diff --git a/dts/tests/TestSuite_pmd_buffer_scatter.py b/dts/tests/TestSuite_pmd_buffer_scatter.py
index 3701c47408..4cdbdc4272 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]
 from scapy.utils import hexstr  # type: ignore[import]
 
+from framework.params import StrParams
 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_parameters=StrParams(
                 "--mbcache=200 "
                 f"--mbuf-size={mbsize} "
                 "--max-pkt-len=9000 "
-- 
2.34.1