On Tue, Mar 26, 2024 at 8:04 PM Luca Vizzarro <luca.vizza...@arm.com> wrote:
>
> This commit provides a state container for TestPmdShell. It currently
> only indicates whether the packet forwarding has started
> or not, and the number of ports which were given to the shell.
>
A reminder, the commit message should explain why we're doing this
change, not what the change is.
> This also fixes the behaviour of `wait_link_status_up` to use the
> command timeout as inherited from InteractiveShell.
>
> Signed-off-by: Luca Vizzarro <luca.vizza...@arm.com>
> Reviewed-by: Jack Bond-Preston <jack.bond-pres...@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagaraha...@arm.com>
> ---
> dts/framework/remote_session/testpmd_shell.py | 41 +++++++++++++------
> 1 file changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/dts/framework/remote_session/testpmd_shell.py
> b/dts/framework/remote_session/testpmd_shell.py
> index a823dc53be..ea1d254f86 100644
> --- a/dts/framework/remote_session/testpmd_shell.py
> +++ b/dts/framework/remote_session/testpmd_shell.py
> @@ -678,19 +678,27 @@ def __str__(self) -> str:
> return self.pci_address
>
>
> +@dataclass(slots=True)
> +class TestPmdState:
> + """Session state container."""
> +
> + #:
> + packet_forwarding_started: bool = False
The same question as in the previous patch, do you anticipate this
being needed and should we add this only when it's actually used?
> +
> + #: The number of ports which were allowed on the command-line when
> testpmd was started.
> + number_of_ports: int = 0
> +
> +
> class TestPmdShell(InteractiveShell):
> """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.
> -
> - Attributes:
> - number_of_ports: The number of ports which were allowed on the
> command-line when testpmd
> - was started.
> """
>
> - number_of_ports: int
> + #: Current state
> + state: TestPmdState = TestPmdState()
Assigning a value makes this a class variable, shared across all
instances. This should be initialized in __init__().
But do we actually want to do this via composition? We'd need to
access the attributes via .state all the time and I don't really like
that. We could just put them into TestPmdShell directly, initializing
them in __init__().
>
> #: The path to the testpmd executable.
> path: ClassVar[PurePath] = PurePath("app", "dpdk-testpmd")
