On Thu, Nov 23, 2023 at 10:14 AM Juraj Linkeš <juraj.lin...@pantheon.tech>
wrote:
> Format according to the Google format and PEP257, with slight
> deviations.
>
> Signed-off-by: Juraj Linkeš <juraj.lin...@pantheon.tech>
> ---
> .../traffic_generator/__init__.py | 22 ++++++++-
> .../capturing_traffic_generator.py | 45 +++++++++++--------
> .../traffic_generator/traffic_generator.py | 33 ++++++++------
> 3 files changed, 67 insertions(+), 33 deletions(-)
>
> diff --git a/dts/framework/testbed_model/traffic_generator/__init__.py
> b/dts/framework/testbed_model/traffic_generator/__init__.py
> index 52888d03fa..11e2bd7d97 100644
> --- a/dts/framework/testbed_model/traffic_generator/__init__.py
> +++ b/dts/framework/testbed_model/traffic_generator/__init__.py
> @@ -1,6 +1,19 @@
> # SPDX-License-Identifier: BSD-3-Clause
> # Copyright(c) 2023 PANTHEON.tech s.r.o.
>
> +"""DTS traffic generators.
> +
> +A traffic generator is capable of generating traffic and then monitor
> returning traffic.
> +All traffic generators must count the number of received packets. Some
> may additionally capture
> +individual packets.
> +
> +A traffic generator may be software running on generic hardware or it
> could be specialized hardware.
> +
> +The traffic generators that only count the number of received packets are
> suitable only for
> +performance testing. In functional testing, we need to be able to dissect
> each arrived packet
> +and a capturing traffic generator is required.
> +"""
> +
> from framework.config import ScapyTrafficGeneratorConfig,
> TrafficGeneratorType
> from framework.exception import ConfigurationError
> from framework.testbed_model.node import Node
> @@ -12,8 +25,15 @@
> def create_traffic_generator(
> tg_node: Node, traffic_generator_config: ScapyTrafficGeneratorConfig
> ) -> CapturingTrafficGenerator:
> - """A factory function for creating traffic generator object from user
> config."""
> + """The factory function for creating traffic generator objects from
> the test run configuration.
> +
> + Args:
> + tg_node: The traffic generator node where the created traffic
> generator will be running.
> + traffic_generator_config: The traffic generator config.
>
> + Returns:
> + A traffic generator capable of capturing received packets.
> + """
> match traffic_generator_config.traffic_generator_type:
> case TrafficGeneratorType.SCAPY:
> return ScapyTrafficGenerator(tg_node,
> traffic_generator_config)
> diff --git
> a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> index 1fc7f98c05..0246590333 100644
> ---
> a/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> +++
> b/dts/framework/testbed_model/traffic_generator/capturing_traffic_generator.py
> @@ -23,19 +23,21 @@
>
>
> def _get_default_capture_name() -> str:
> - """
> - This is the function used for the default implementation of capture
> names.
> - """
> return str(uuid.uuid4())
>
>
> class CapturingTrafficGenerator(TrafficGenerator):
> """Capture packets after sending traffic.
>
> - A mixin interface which enables a packet generator to declare that it
> can capture
> + The intermediary interface which enables a packet generator to
> declare that it can capture
> packets and return them to the user.
>
> + Similarly to :class:`~.traffic_generator.TrafficGenerator`, this
> class exposes
> + the public methods specific to capturing traffic generators and
> defines a private method
> + that must implement the traffic generation and capturing logic in
> subclasses.
> +
> The methods of capturing traffic generators obey the following
> workflow:
> +
> 1. send packets
> 2. capture packets
> 3. write the capture to a .pcap file
> @@ -44,6 +46,7 @@ class CapturingTrafficGenerator(TrafficGenerator):
>
> @property
> def is_capturing(self) -> bool:
> + """This traffic generator can capture traffic."""
> return True
>
> def send_packet_and_capture(
> @@ -54,11 +57,12 @@ def send_packet_and_capture(
> duration: float,
> capture_name: str = _get_default_capture_name(),
> ) -> list[Packet]:
> - """Send a packet, return received traffic.
> + """Send `packet` and capture received traffic.
> +
> + Send `packet` on `send_port` and then return all traffic captured
> + on `receive_port` for the given `duration`.
>
> - Send a packet on the send_port and then return all traffic
> captured
> - on the receive_port for the given duration. Also record the
> captured traffic
> - in a pcap file.
> + The captured traffic is recorded in the `capture_name`.pcap file.
>
> Args:
> packet: The packet to send.
> @@ -68,7 +72,7 @@ def send_packet_and_capture(
> capture_name: The name of the .pcap file where to store the
> capture.
>
> Returns:
> - A list of received packets. May be empty if no packets are
> captured.
> + The received packets. May be empty if no packets are
> captured.
> """
> return self.send_packets_and_capture(
> [packet], send_port, receive_port, duration, capture_name
> @@ -82,11 +86,14 @@ def send_packets_and_capture(
> duration: float,
> capture_name: str = _get_default_capture_name(),
> ) -> list[Packet]:
> - """Send packets, return received traffic.
> + """Send `packets` and capture received traffic.
>
> - Send packets on the send_port and then return all traffic captured
> - on the receive_port for the given duration. Also record the
> captured traffic
> - in a pcap file.
> + Send `packets` on `send_port` and then return all traffic captured
> + on `receive_port` for the given `duration`.
> +
> + The captured traffic is recorded in the `capture_name`.pcap file.
> The target directory
> + can be configured with the :option:`--output-dir` command line
> argument or
> + the :envvar:`DTS_OUTPUT_DIR` environment variable.
>
> Args:
> packets: The packets to send.
> @@ -96,7 +103,7 @@ def send_packets_and_capture(
> capture_name: The name of the .pcap file where to store the
> capture.
>
> Returns:
> - A list of received packets. May be empty if no packets are
> captured.
> + The received packets. May be empty if no packets are
> captured.
> """
> self._logger.debug(get_packet_summaries(packets))
> self._logger.debug(
> @@ -121,10 +128,12 @@ def _send_packets_and_capture(
> receive_port: Port,
> duration: float,
> ) -> list[Packet]:
> - """
> - The extended classes must implement this method which
> - sends packets on send_port and receives packets on the
> receive_port
> - for the specified duration. It must be able to handle no received
> packets.
> + """The implementation of :method:`send_packets_and_capture`.
> +
> + The subclasses must implement this method which sends `packets`
> on `send_port`
> + and receives packets on `receive_port` for the specified
> `duration`.
> +
> + It must be able to handle receiving no packets.
> """
>
> def _write_capture_from_packets(self, capture_name: str, packets:
> list[Packet]) -> None:
> diff --git
> a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
> b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
> index 0d9902ddb7..5fb9824568 100644
> --- a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
> +++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
> @@ -22,7 +22,8 @@
> class TrafficGenerator(ABC):
> """The base traffic generator.
>
> - Defines the few basic methods that each traffic generator must
> implement.
> + Exposes the common public methods of all traffic generators and
> defines private methods
> + that must implement the traffic generation logic in subclasses.
> """
>
> _config: TrafficGeneratorConfig
> @@ -30,14 +31,20 @@ class TrafficGenerator(ABC):
> _logger: DTSLOG
>
> def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
> + """Initialize the traffic generator.
> +
> + Args:
> + tg_node: The traffic generator node where the created traffic
> generator will be running.
> + config: The traffic generator's test run configuration.
> + """
> self._config = config
> self._tg_node = tg_node
> self._logger = getLogger(f"{self._tg_node.name}
> {self._config.traffic_generator_type}")
>
> def send_packet(self, packet: Packet, port: Port) -> None:
> - """Send a packet and block until it is fully sent.
> + """Send `packet` and block until it is fully sent.
>
> - What fully sent means is defined by the traffic generator.
> + Send `packet` on `port`, then wait until `packet` is fully sent.
>
> Args:
> packet: The packet to send.
> @@ -46,9 +53,9 @@ def send_packet(self, packet: Packet, port: Port) ->
> None:
> self.send_packets([packet], port)
>
> def send_packets(self, packets: list[Packet], port: Port) -> None:
> - """Send packets and block until they are fully sent.
> + """Send `packets` and block until they are fully sent.
>
> - What fully sent means is defined by the traffic generator.
> + Send `packets` on `port`, then wait until `packets` are fully
> sent.
>
> Args:
> packets: The packets to send.
> @@ -60,19 +67,17 @@ def send_packets(self, packets: list[Packet], port:
> Port) -> None:
>
> @abstractmethod
> def _send_packets(self, packets: list[Packet], port: Port) -> None:
> - """
> - The extended classes must implement this method which
> - sends packets on send_port. The method should block until all
> packets
> - are fully sent.
> + """The implementation of :method:`send_packets`.
> +
> + The subclasses must implement this method which sends `packets`
> on `port`.
> + The method should block until all `packets` are fully sent.
> +
> + What full sent means is defined by the traffic generator.
> """
>
I think this should be "what fully sent means"
> @property
> def is_capturing(self) -> bool:
> - """Whether this traffic generator can capture traffic.
> -
> - Returns:
> - True if the traffic generator can capture traffic, False
> otherwise.
> - """
> + """This traffic generator can't capture traffic."""
> return False
>
> @abstractmethod
> --
> 2.34.1
>
>
