From b0a8bfce930fc27f4c1fa6a065fecf4c8891e127 Mon Sep 17 00:00:00 2001 From: Theo Clementz Date: Thu, 14 Nov 2024 14:43:01 +0100 Subject: [PATCH 1/3] fix: Make it possible to 'open' again a 'CCTcpip' that was previously closed --- src/pykiso/lib/connectors/cc_tcp_ip.py | 14 +++++++++++--- tests/test_cc_tcp_ip.py | 8 ++++++-- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/src/pykiso/lib/connectors/cc_tcp_ip.py b/src/pykiso/lib/connectors/cc_tcp_ip.py index 4eaf40866..9c12785e3 100644 --- a/src/pykiso/lib/connectors/cc_tcp_ip.py +++ b/src/pykiso/lib/connectors/cc_tcp_ip.py @@ -41,7 +41,7 @@ def __init__(self, dest_ip: str, dest_port: int, max_msg_size: int = 256, **kwar super().__init__(**kwargs) self.dest_ip = dest_ip self.dest_port = int(dest_port) - self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + self.socket: socket.socket | None = None self.max_msg_size = max_msg_size # Set a timeout to send the signal to the GIL to change thread. # In case of a multi-threading system, all tasks will be called one after the other. @@ -50,13 +50,16 @@ def __init__(self, dest_ip: str, dest_port: int, max_msg_size: int = 256, **kwar def _cc_open(self) -> None: """Connect to socket with configured port and IP address.""" log.internal_info(f"Connection to socket at address {self.dest_ip} port {self.dest_port}") + self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) self.socket.settimeout(3) self.socket.connect((self.dest_ip, self.dest_port)) def _cc_close(self) -> None: """Close UDP socket.""" - log.internal_info(f"Disconnect from socket at address {self.dest_ip}, port {self.dest_port}") - self.socket.close() + if self.socket: + log.internal_info(f"Disconnect from socket at address {self.dest_ip}, port {self.dest_port}") + self.socket.close() + self.socket = None def _cc_send(self, msg: bytes or str, **kwargs) -> None: """Send a message via socket. @@ -64,6 +67,8 @@ def _cc_send(self, msg: bytes or str, **kwargs) -> None: :param msg: message to send :param kwargs: not used """ + if not self.socket: + raise TypeError("Channel must be opened before messages can be sent") if isinstance(msg, str): msg = msg.encode() log.internal_debug(f"Sending {msg} via socket to {self.dest_ip}") @@ -76,6 +81,9 @@ def _cc_receive(self, timeout=0.01) -> Dict[str, Optional[bytes]]: :return: Message if successful, otherwise none """ + if not self.socket: + raise TypeError("Channel must be opened before messages can be received") + self.socket.settimeout(timeout or self.timeout) try: diff --git a/tests/test_cc_tcp_ip.py b/tests/test_cc_tcp_ip.py index d4b94b396..5746629b5 100644 --- a/tests/test_cc_tcp_ip.py +++ b/tests/test_cc_tcp_ip.py @@ -68,7 +68,6 @@ def test_constructor(constructor_params): assert socket_connector.dest_ip == constructor_params["ip"] assert socket_connector.dest_port == constructor_params["port"] assert socket_connector.max_msg_size == constructor_params["max_msg_size"] - assert isinstance(socket_connector.socket, socket.socket) assert socket_connector.timeout == 1e-6 @@ -97,7 +96,7 @@ def test__cc_close(mock_socket, constructor_params): """Test _cc_close""" param = constructor_params.values() socket_connector = cc_tcp_ip.CCTcpip(*param) - + socket_connector.open() socket_connector._cc_close() mock_socket.socket.socket.close.assert_called_once() @@ -118,6 +117,7 @@ def test__cc_send( """Test _cc_send""" param = constructor_params.values() socket_connector = cc_tcp_ip.CCTcpip(*param) + socket_connector.open() socket_connector._cc_send(msg_to_send) mock_socket.socket.socket.send.assert_called_once_with(expected_sent_message) @@ -137,6 +137,9 @@ def test__cc_receive( """Test _cc_receive""" param = constructor_params.values() socket_connector = cc_tcp_ip.CCTcpip(*param) + socket_connector.open() + + socket_connector.socket.settimeout.reset_mock() response = socket_connector._cc_receive(timeout=timeout) if not is_raw: response["msg"] = response.get("msg").decode().strip() @@ -156,6 +159,7 @@ def test__cc_receive_with_errors( """Test _cc_receive with errors""" param = constructor_params.values() socket_connector = cc_tcp_ip.CCTcpip(*param) + socket_connector.open() # using max_msg_size attribute tu trigger exceptions in mock for err in errors_to_catch: From a072be72ebd4dfa6022358531763e1b5a58f9003 Mon Sep 17 00:00:00 2001 From: Theo Clementz Date: Thu, 14 Nov 2024 16:26:09 +0100 Subject: [PATCH 2/3] don't create a new socket if one is already in use --- src/pykiso/lib/connectors/cc_tcp_ip.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pykiso/lib/connectors/cc_tcp_ip.py b/src/pykiso/lib/connectors/cc_tcp_ip.py index 9c12785e3..4e0080b5f 100644 --- a/src/pykiso/lib/connectors/cc_tcp_ip.py +++ b/src/pykiso/lib/connectors/cc_tcp_ip.py @@ -49,6 +49,8 @@ def __init__(self, dest_ip: str, dest_port: int, max_msg_size: int = 256, **kwar def _cc_open(self) -> None: """Connect to socket with configured port and IP address.""" + if self.socket: + return log.internal_info(f"Connection to socket at address {self.dest_ip} port {self.dest_port}") self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) self.socket.settimeout(3) From 1cec93df7902c00e114a72d49011d36075a922a8 Mon Sep 17 00:00:00 2001 From: Theo Clementz Date: Fri, 15 Nov 2024 09:53:21 +0100 Subject: [PATCH 3/3] Apply code review suggestions (Python 3.9 compatibility, exception type, warning when reopenging connector) --- src/pykiso/lib/connectors/cc_tcp_ip.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/pykiso/lib/connectors/cc_tcp_ip.py b/src/pykiso/lib/connectors/cc_tcp_ip.py index 4e0080b5f..f1ee8a08c 100644 --- a/src/pykiso/lib/connectors/cc_tcp_ip.py +++ b/src/pykiso/lib/connectors/cc_tcp_ip.py @@ -41,7 +41,7 @@ def __init__(self, dest_ip: str, dest_port: int, max_msg_size: int = 256, **kwar super().__init__(**kwargs) self.dest_ip = dest_ip self.dest_port = int(dest_port) - self.socket: socket.socket | None = None + self.socket: Optional[socket.socket] = None self.max_msg_size = max_msg_size # Set a timeout to send the signal to the GIL to change thread. # In case of a multi-threading system, all tasks will be called one after the other. @@ -50,6 +50,10 @@ def __init__(self, dest_ip: str, dest_port: int, max_msg_size: int = 256, **kwar def _cc_open(self) -> None: """Connect to socket with configured port and IP address.""" if self.socket: + log.internal_info( + "`CCTcpip._cc_open` called again; already connected " + f"to socket at address {self.dest_ip} port {self.dest_port}" + ) return log.internal_info(f"Connection to socket at address {self.dest_ip} port {self.dest_port}") self.socket = socket.socket(socket.AF_INET, socket.SOCK_STREAM) @@ -70,7 +74,7 @@ def _cc_send(self, msg: bytes or str, **kwargs) -> None: :param kwargs: not used """ if not self.socket: - raise TypeError("Channel must be opened before messages can be sent") + raise RuntimeError("Channel must be opened before messages can be sent") if isinstance(msg, str): msg = msg.encode() log.internal_debug(f"Sending {msg} via socket to {self.dest_ip}") @@ -84,7 +88,7 @@ def _cc_receive(self, timeout=0.01) -> Dict[str, Optional[bytes]]: :return: Message if successful, otherwise none """ if not self.socket: - raise TypeError("Channel must be opened before messages can be received") + raise RuntimeError("Channel must be opened before messages can be received") self.socket.settimeout(timeout or self.timeout)