From d0c5ef7dd124443675e98b1b062e039ac6676ee5 Mon Sep 17 00:00:00 2001 From: Dominic Reber <71256590+domire8@users.noreply.github.com> Date: Fri, 6 Oct 2023 09:12:27 +0200 Subject: [PATCH] feat: add is_opened flag to socket base class (#68) --- .devcontainer/devcontainer.json | 22 ++++++++++ .gitignore | 1 + .vscode/c_cpp_properties.json | 15 +++++++ .vscode/settings.json | 5 +++ CHANGELOG.md | 4 ++ README.md | 25 +++++------ build.sh | 16 ++++---- .../src/communication_interfaces_bindings.cpp | 15 +------ python/test/test_tcp.py | 25 +++++++++++ python/test/test_udp.py | 33 +++++++++++++-- python/test/test_zmq.py | 16 ++++++++ .../communication_interfaces/CMakeLists.txt | 1 + .../sockets/ISocket.hpp | 41 +++++++++++++++++-- .../sockets/TCPClient.hpp | 6 +-- .../sockets/TCPServer.hpp | 11 ++--- .../sockets/TCPSocket.hpp | 18 ++++---- .../sockets/UDPClient.hpp | 13 +++--- .../sockets/UDPServer.hpp | 14 +++---- .../sockets/UDPSocket.hpp | 10 ++--- .../sockets/ZMQPublisher.hpp | 7 ++-- .../sockets/ZMQPublisherSubscriber.hpp | 12 +++--- .../sockets/ZMQSocket.hpp | 31 +++++++------- .../sockets/ZMQSubscriber.hpp | 6 +-- .../src/sockets/ISocket.cpp | 30 ++++++++++++++ .../src/sockets/TCPClient.cpp | 2 +- .../src/sockets/TCPServer.cpp | 8 ++-- .../src/sockets/TCPSocket.cpp | 8 ++-- .../src/sockets/UDPClient.cpp | 6 +-- .../src/sockets/UDPServer.cpp | 6 +-- .../src/sockets/UDPSocket.cpp | 4 +- .../src/sockets/ZMQPublisher.cpp | 4 +- .../src/sockets/ZMQPublisherSubscriber.cpp | 8 ++-- .../src/sockets/ZMQSocket.cpp | 8 ++-- .../src/sockets/ZMQSubscriber.cpp | 4 +- .../test/tests/test_tcp_communication.cpp | 18 ++++++++ .../test/tests/test_udp_communication.cpp | 24 ++++++++--- .../test/tests/test_zmq_communication.cpp | 12 ++++++ 37 files changed, 352 insertions(+), 137 deletions(-) create mode 100644 .devcontainer/devcontainer.json create mode 100644 .vscode/c_cpp_properties.json create mode 100644 .vscode/settings.json create mode 100644 source/communication_interfaces/src/sockets/ISocket.cpp diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json new file mode 100644 index 0000000..fb1002a --- /dev/null +++ b/.devcontainer/devcontainer.json @@ -0,0 +1,22 @@ +{ + "name": "network interfaces", + "remoteUser": "ros2", + "build": { + "dockerfile": "../Dockerfile", + "context": "..", + "target": "development", + "args": { + "ROS2_VERSION": "iron" + } + }, + "workspaceMount": "source=${localWorkspaceFolder},target=/home/ros2/.devcontainer,type=bind,consistency=cached", + "workspaceFolder": "/home/ros2/.devcontainer", + "customizations": { + "vscode": { + "extensions": [ + "ms-vscode.cpptools-extension-pack", + "eamodio.gitlens" + ] + } + } +} \ No newline at end of file diff --git a/.gitignore b/.gitignore index 2a2c876..effd9a6 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ *.idea *cmake-build-debug* +build/ diff --git a/.vscode/c_cpp_properties.json b/.vscode/c_cpp_properties.json new file mode 100644 index 0000000..6efa267 --- /dev/null +++ b/.vscode/c_cpp_properties.json @@ -0,0 +1,15 @@ +{ + "configurations": [ + { + "name": "AMD", + "includePath": [ + "/home/ros2/.devcontainer/source/communication_interfaces/include/**" + ], + "compilerPath": "/usr/bin/gcc", + "cStandard": "c17", + "cppStandard": "gnu++17", + "intelliSenseMode": "linux-gcc-x64" + } + ], + "version": 4 +} \ No newline at end of file diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 0000000..b9ff247 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,5 @@ +{ + "cmake.sourceDirectory": "/home/ros2/.devcontainer/source/communication_interfaces", + "cmake.configureArgs": [ "-DBUILD_TESTING=ON" ], + "C_Cpp.clang_format_style": "file:/home/ros2/.clang-format" +} \ No newline at end of file diff --git a/CHANGELOG.md b/CHANGELOG.md index afb82d3..f30bb9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,10 @@ Release Versions: - [0.2.0](#020) - [0.1.0](#010) +## Upcoming changes (in development) + +- feat: add is_opened flag to socket base class (#68) + ## 2.0.1 Version 2.0.1 contains a hotfix that enables socket communiction with any serialized message in Python, which was not diff --git a/README.md b/README.md index f5395b1..2e3d3a4 100644 --- a/README.md +++ b/README.md @@ -21,12 +21,12 @@ respectively. #### Implementing a derived socket class To use this class, create a subclass that inherits from it and implement its pure virtual functions. The pure virtual -functions are `open()`, `receive_bytes(std::string&)`, and `send_bytes(const std::string&)`. +functions are `on_open()`, `on_receive_bytes(std::string&)`, and `on_send_bytes(const std::string&)`. Configuration parameters should be passed with a configuration struct, resulting in a single argument constructor. -The `close()` function can optionally be overridden to perform steps to disconnect and close the socket communication. -If a derived class defines any cleanup behavior in `close()`, it should also be invoked statically and explicitly +The `on_close()` function can optionally be overridden to perform steps to disconnect and close the socket communication. +If a derived class defines any cleanup behavior in `on_close()`, it should also be invoked statically and explicitly in the destructor of the derived class. An example is given below. @@ -45,13 +45,14 @@ public: ~DerivedSocket() override; - void open() override; +private: + void on_open() override; - bool receive_bytes(std::string& buffer) override; + bool on_receive_bytes(std::string& buffer) override; - bool send_bytes(const std::string& buffer) override; + bool on_send_bytes(const std::string& buffer) override; - void close() override; + void on_close() override; } ``` @@ -62,24 +63,24 @@ DerivedSocket::DerivedSocket(DerivedSocketConfig configuraiton) { } DerivedSocket::~DerivedSocket() { - DerivedSocket::close(); + DerivedSocket::on_close(); } -void DerivedSocket::open() { +void DerivedSocket::on_open() { // Configure and open the socket } -bool DerivedSocket::receive_bytes(std::string& buffer) { +bool DerivedSocket::on_receive_bytes(std::string& buffer) { // Read the contents of the socket into the buffer and return true on success. Otherwise, return false. return true; } -bool DerivedSocket::send_bytes(const std::string& buffer) { +bool DerivedSocket::on_send_bytes(const std::string& buffer) { // Write the contents of the buffer onto the socket and return true on success. Otherwise, return false. return true; } -void DerivedSocket::close() { +void DerivedSocket::on_close() { // Perform clean-up steps here } ``` diff --git a/build.sh b/build.sh index 341e27b..748efe4 100644 --- a/build.sh +++ b/build.sh @@ -7,19 +7,19 @@ ROS2_VERSION=humble HELP_MESSAGE="Usage: build.sh [options] Options: - --test Target the test layer to run the tests. + --test Target the test layer to run the tests. - --ros2-version Specify the version of ROS 2 to use. - (default: $ROS2_VERSION) + --ros2-version Specify the version of ROS 2 to use. + (default: $ROS2_VERSION) - -v|--verbose Set the build output to verbose. + -v|--verbose Set the build output to verbose. - --cache-id Invalidate the mount cache (e.g. CMake build folder) - by providing a new value. + --cache-id Invalidate the mount cache (e.g. CMake build folder) + by providing a new value. - -r|--no-cache Invalidate all cache (layer + mount). + -r|--no-cache Invalidate all cache (layer + mount). - -h|--help Show this help message. + -h|--help Show this help message. " TEST=0 diff --git a/python/src/communication_interfaces_bindings.cpp b/python/src/communication_interfaces_bindings.cpp index 8bade4d..910f2e9 100644 --- a/python/src/communication_interfaces_bindings.cpp +++ b/python/src/communication_interfaces_bindings.cpp @@ -8,19 +8,6 @@ using namespace communication_interfaces; -class PySocket : public sockets::ISocket, public std::enable_shared_from_this { -public: - using sockets::ISocket::ISocket; - - void open() override { PYBIND11_OVERRIDE_PURE(void, ISocket, open, ); } - - bool receive_bytes(std::string& buffer) override { PYBIND11_OVERRIDE_PURE(bool, ISocket, receive_bytes, buffer); } - - bool send_bytes(const std::string& buffer) override { PYBIND11_OVERRIDE_PURE(bool, ISocket, send_bytes, buffer); } - - void close() override { PYBIND11_OVERRIDE(void, ISocket, close, ); } -}; - PYBIND11_MODULE(communication_interfaces, m) { m.doc() = "Python bindings for communication interfaces"; @@ -36,7 +23,7 @@ PYBIND11_MODULE(communication_interfaces, m) { auto m_sub_sock = m.def_submodule("sockets", "Submodule for communication interfaces sockets"); - py::class_, PySocket>(m_sub_sock, "ISocket") + py::class_>(m_sub_sock, "ISocket") .def("open", &sockets::ISocket::open, "Perform configuration steps to open the socket for communication") .def( "receive_bytes", diff --git a/python/test/test_tcp.py b/python/test/test_tcp.py index 00d3bdf..8a9ef27 100644 --- a/python/test/test_tcp.py +++ b/python/test/test_tcp.py @@ -2,6 +2,7 @@ import pytest import time +from communication_interfaces.exceptions import SocketConfigurationError from communication_interfaces.sockets import TCPClientConfiguration, TCPClient, TCPServerConfiguration, TCPServer @@ -29,3 +30,27 @@ def test_comm(self): response = self.client.receive_bytes() assert response assert response.decode("utf-8").rstrip("\x00") == self.server_message + + buffer = "" + self.server.close() + with pytest.raises(SocketConfigurationError): + self.server.receive_bytes() + with pytest.raises(SocketConfigurationError): + self.server.send_bytes(buffer) + self.client.close() + with pytest.raises(SocketConfigurationError): + self.client.receive_bytes() + with pytest.raises(SocketConfigurationError): + self.client.send_bytes(buffer) + + def test_not_open(self): + buffer = "" + with pytest.raises(SocketConfigurationError): + self.server.receive_bytes() + with pytest.raises(SocketConfigurationError): + self.server.send_bytes(buffer) + + with pytest.raises(SocketConfigurationError): + self.client.receive_bytes() + with pytest.raises(SocketConfigurationError): + self.client.send_bytes(buffer) diff --git a/python/test/test_udp.py b/python/test/test_udp.py index d5846f5..8cbeeb5 100644 --- a/python/test/test_udp.py +++ b/python/test/test_udp.py @@ -60,8 +60,35 @@ def test_port_reuse(udp_config, server): server2.open() -def test_open_close(server): +def test_open_close(udp_config): + buffer = "" + udp_config.timeout_duration_sec = 0.5 + server = UDPServer(udp_config) + with pytest.raises(SocketConfigurationError): + server.send_bytes(buffer) + with pytest.raises(SocketConfigurationError): + server.receive_bytes() server.open() - server.close() + assert server.send_bytes("test") + assert not server.receive_bytes() - assert not server.send_bytes("") + server.close() + with pytest.raises(SocketConfigurationError): + server.send_bytes(buffer) + with pytest.raises(SocketConfigurationError): + server.receive_bytes() + + client = UDPClient(udp_config) + with pytest.raises(SocketConfigurationError): + client.send_bytes(buffer) + with pytest.raises(SocketConfigurationError): + client.receive_bytes() + client.open() + assert client.send_bytes("test") + assert not client.receive_bytes() + + client.close() + with pytest.raises(SocketConfigurationError): + client.send_bytes(buffer) + with pytest.raises(SocketConfigurationError): + client.receive_bytes() diff --git a/python/test/test_zmq.py b/python/test/test_zmq.py index f2511ae..7911ff5 100644 --- a/python/test/test_zmq.py +++ b/python/test/test_zmq.py @@ -1,6 +1,7 @@ import pytest import time +from communication_interfaces.exceptions import SocketConfigurationError from communication_interfaces.sockets import ZMQContext, ZMQSocketConfiguration, ZMQPublisher, ZMQSubscriber, \ ZMQCombinedSocketsConfiguration, ZMQPublisherSubscriber @@ -71,3 +72,18 @@ def test_send_receive_combined(zmq_context): server.close() client.close() + + +def test_open_close(zmq_config): + buffer = "" + socket = ZMQPublisher(zmq_config) + with pytest.raises(SocketConfigurationError): + socket.send_bytes(buffer) + with pytest.raises(SocketConfigurationError): + socket.receive_bytes() + + socket.open() + assert socket.send_bytes("test") + socket.close() + with pytest.raises(SocketConfigurationError): + socket.send_bytes(buffer) diff --git a/source/communication_interfaces/CMakeLists.txt b/source/communication_interfaces/CMakeLists.txt index 461bc2a..be7ea53 100644 --- a/source/communication_interfaces/CMakeLists.txt +++ b/source/communication_interfaces/CMakeLists.txt @@ -52,6 +52,7 @@ if (NOT cppzmq_POPULATED) endif() add_library(${PROJECT_NAME} SHARED + ${PROJECT_SOURCE_DIR}/src/sockets/ISocket.cpp ${PROJECT_SOURCE_DIR}/src/sockets/UDPSocket.cpp ${PROJECT_SOURCE_DIR}/src/sockets/UDPClient.cpp ${PROJECT_SOURCE_DIR}/src/sockets/UDPServer.cpp diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/ISocket.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/ISocket.hpp index 2badfe7..1a2db3e 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/ISocket.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/ISocket.hpp @@ -2,6 +2,8 @@ #include +#include "communication_interfaces/exceptions/SocketConfigurationException.hpp" + namespace communication_interfaces::sockets { /** @@ -23,25 +25,56 @@ class ISocket { * @brief Perform configuration steps to open the socket for communication * @throws SocketConfigurationException if opening fails */ - virtual void open() = 0; + void open(); /** * @brief Receive bytes from the socket * @param buffer The buffer to fill with the received bytes * @return True if bytes were received, false otherwise + * @throws SocketConfigurationException if socket has not been opened yet */ - virtual bool receive_bytes(std::string& buffer) = 0; + bool receive_bytes(std::string& buffer); /** * @brief Send bytes to the socket * @param buffer The buffer with the bytes to send * @return True if bytes were sent, false otherwise + * @throws SocketConfigurationException if socket has not been opened yet */ - virtual bool send_bytes(const std::string& buffer) = 0; + bool send_bytes(const std::string& buffer); /** * @brief Perform steps to disconnect and close the socket communication */ - virtual void close() {} + void close(); + +protected: + /** + * @brief Perform configuration steps to open the socket for communication + * @throws SocketConfigurationException if opening fails + */ + virtual void on_open() = 0; + + /** + * @brief Receive bytes from the socket + * @param buffer The buffer to fill with the received bytes + * @return True if bytes were received, false otherwise + */ + virtual bool on_receive_bytes(std::string& buffer) = 0; + + /** + * @brief Send bytes to the socket + * @param buffer The buffer with the bytes to send + * @return True if bytes were sent, false otherwise + */ + virtual bool on_send_bytes(const std::string& buffer) = 0; + + /** + * @brief Perform steps to disconnect and close the socket communication + */ + virtual void on_close() {}; + +private: + bool opened_ = false; }; }// namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/TCPClient.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/TCPClient.hpp index e4e7ed0..6404807 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/TCPClient.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/TCPClient.hpp @@ -22,13 +22,13 @@ class TCPClient : public TCPSocket { */ explicit TCPClient(TCPClientConfiguration configuration); +private: /** - * @copydoc ISocket::open() + * @copydoc ISocket::on_open() * @details Connect the client socket to the server */ - void open() override; + void on_open() override; -private: TCPClientConfiguration config_; ///< Socket configuration struct }; } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/TCPServer.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/TCPServer.hpp index 78c8c2c..b3ebd3a 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/TCPServer.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/TCPServer.hpp @@ -27,19 +27,20 @@ class TCPServer : public TCPSocket { */ ~TCPServer() override; +private: /** - * @copydoc ISocket::open() + * @copydoc ISocket::on_open() * @details Wait for connection requests from clients and accept new connections. This method blocks until a * connection is established */ - void open() override; + void on_open() override; /** - * @brief Close the sockets + * @copydoc ISocket::on_close() */ - void close() override; + void on_close() override; + -private: TCPServerConfiguration config_; ///< Socket configuration struct int server_fd_; ///< File descriptor of the connected server socket }; diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/TCPSocket.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/TCPSocket.hpp index 8ceba43..9eea886 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/TCPSocket.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/TCPSocket.hpp @@ -18,23 +18,23 @@ class TCPSocket : public ISocket { */ ~TCPSocket() override; +protected: + explicit TCPSocket(int buffer_size); + /** - * @copydoc ISocket::receive_bytes(std::string&) + * @copydoc ISocket::on_receive_bytes(std::string&) */ - bool receive_bytes(std::string& buffer) override; + bool on_receive_bytes(std::string& buffer) override; /** - * @copydoc ISocket::receive_bytes(std::string&) + * @copydoc ISocket::on_receive_bytes(std::string&) */ - bool send_bytes(const std::string& buffer) override; + bool on_send_bytes(const std::string& buffer) override; /** - * @brief Close the socket + * @copydoc ISocket::on_close(std::string&) */ - void close() override; - -protected: - explicit TCPSocket(int buffer_size); + void on_close() override; sockaddr_in server_address_; ///< Address of the TCP server int socket_fd_; ///< File descriptor of the socket diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/UDPClient.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/UDPClient.hpp index e27ecff..0513bcb 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/UDPClient.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/UDPClient.hpp @@ -15,19 +15,20 @@ class UDPClient : public UDPSocket { */ UDPClient(UDPSocketConfiguration configuration); +private: /** - * @copydoc ISocket::open() + * @copydoc ISocket::on_open() */ - void open() override; + void on_open() override; /** - * @copydoc ISocket::receive_bytes(std::string&) + * @copydoc ISocket::on_receive_bytes(std::string&) */ - bool receive_bytes(std::string& buffer) override; + bool on_receive_bytes(std::string& buffer) override; /** - * @copydoc ISocket::send_bytes(const std::string&) + * @copydoc ISocket::on_send_bytes(const std::string&) */ - bool send_bytes(const std::string& buffer) override; + bool on_send_bytes(const std::string& buffer) override; }; } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/UDPServer.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/UDPServer.hpp index ce39a46..465fbba 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/UDPServer.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/UDPServer.hpp @@ -15,22 +15,22 @@ class UDPServer : public UDPSocket { */ UDPServer(UDPSocketConfiguration configuration); +private: /** - * @copydoc ISocket::open() + * @copydoc ISocket::on_open() */ - void open() override; + void on_open() override; /** - * @copydoc ISocket::receive_bytes(std::string&) + * @copydoc ISocket::on_receive_bytes(std::string&) */ - bool receive_bytes(std::string& buffer) override; + bool on_receive_bytes(std::string& buffer) override; /** - * @copydoc ISocket::send_bytes(const std::string&) + * @copydoc ISocket::on_send_bytes(const std::string&) */ - bool send_bytes(const std::string& buffer) override; + bool on_send_bytes(const std::string& buffer) override; -private: sockaddr_in client_address_; }; } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/UDPSocket.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/UDPSocket.hpp index f5bc55d..1036866 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/UDPSocket.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/UDPSocket.hpp @@ -29,11 +29,6 @@ class UDPSocket : public ISocket { */ ~UDPSocket() override; - /** - * @brief Close the socket - */ - void close() override; - protected: /** * @brief Constructor taking the configuration struct @@ -63,6 +58,11 @@ class UDPSocket : public ISocket { */ [[nodiscard]] bool sendto(const sockaddr_in& address, const std::string& buffer) const; + /** + * @copydoc ISocket::on_close() + */ + void on_close() override; + sockaddr_in server_address_; ///< Address of the UDP server private: diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/ZMQPublisher.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/ZMQPublisher.hpp index 1fdc38c..9bba2cd 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/ZMQPublisher.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/ZMQPublisher.hpp @@ -15,14 +15,15 @@ class ZMQPublisher : public ZMQSocket { */ explicit ZMQPublisher(ZMQSocketConfiguration configuration); +private: /** - * @copydoc ISocket::open() + * @copydoc ISocket::on_open() */ - void open() override; + void on_open() override; /** * @brief This method throws a runtime error as receiving is not available for a ZMQ publisher */ - bool receive_bytes(std::string& buffer) override; + bool on_receive_bytes(std::string& buffer) override; }; } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/ZMQPublisherSubscriber.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/ZMQPublisherSubscriber.hpp index fcadd94..dc24ebf 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/ZMQPublisherSubscriber.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/ZMQPublisherSubscriber.hpp @@ -36,32 +36,32 @@ class ZMQPublisherSubscriber : public ISocket { */ ~ZMQPublisherSubscriber() override; +private: /** * @brief Open the internal ZMQ Publisher and Subscriber sockets for communication * @throws SocketConfigurationException if opening fails */ - void open() override; + void on_open() override; /** * @brief Receive bytes from the internal ZMQ Subscriber socket * @param buffer The buffer to fill with the received bytes * @return True if bytes were received, false otherwise */ - bool receive_bytes(std::string& buffer) override; + bool on_receive_bytes(std::string& buffer) override; /** * @brief Send bytes with the internal ZMQ Publisher socket * @param buffer The buffer with the bytes to send * @return True if bytes were sent, false otherwise */ - bool send_bytes(const std::string& buffer) override; + bool on_send_bytes(const std::string& buffer) override; /** - * @brief Close the sockets + * @copydoc ISocket::on_close() */ - void close() override; + void on_close() override; -private: std::shared_ptr pub_; ///< ZMQ publisher std::shared_ptr sub_; ///< ZMQ subscriber }; diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/ZMQSocket.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/ZMQSocket.hpp index 433dcb2..213eb59 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/ZMQSocket.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/ZMQSocket.hpp @@ -29,21 +29,6 @@ class ZMQSocket : public ISocket { */ ~ZMQSocket() override; - /** - * @brief Close the socket - */ - void close() override; - - /** - * @copydoc ISocket::receive_bytes(std::string&) - */ - bool receive_bytes(std::string& buffer) override; - - /** - * @copydoc ISocket::send_bytes(const std::string&) - */ - bool send_bytes(const std::string& buffer) override; - protected: /** * @brief Constructor taking the configuration struct @@ -58,5 +43,21 @@ class ZMQSocket : public ISocket { ZMQSocketConfiguration config_; ///< Socket configuration struct std::shared_ptr socket_; ///< ZMQ socket + +private: + /** + * @copydoc ISocket::on_receive_bytes(std::string&) + */ + bool on_receive_bytes(std::string& buffer) override; + + /** + * @copydoc ISocket::on_send_bytes(const std::string&) + */ + bool on_send_bytes(const std::string& buffer) override; + + /** + * @copydoc ISocket::on_close() + */ + void on_close() override; }; } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/include/communication_interfaces/sockets/ZMQSubscriber.hpp b/source/communication_interfaces/include/communication_interfaces/sockets/ZMQSubscriber.hpp index fd4a62c..9de008c 100644 --- a/source/communication_interfaces/include/communication_interfaces/sockets/ZMQSubscriber.hpp +++ b/source/communication_interfaces/include/communication_interfaces/sockets/ZMQSubscriber.hpp @@ -16,13 +16,13 @@ class ZMQSubscriber : public ZMQSocket { explicit ZMQSubscriber(ZMQSocketConfiguration configuration); /** - * @copydoc ISocket::open() + * @copydoc ISocket::on_open() */ - void open() override; + void on_open() override; /** * @brief This method throws a runtime error as sending is not available for a ZMQ publisher */ - bool send_bytes(const std::string& buffer) override; + bool on_send_bytes(const std::string& buffer) override; }; } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/src/sockets/ISocket.cpp b/source/communication_interfaces/src/sockets/ISocket.cpp new file mode 100644 index 0000000..3af22c8 --- /dev/null +++ b/source/communication_interfaces/src/sockets/ISocket.cpp @@ -0,0 +1,30 @@ +#include "communication_interfaces/sockets/ISocket.hpp" + +#include "communication_interfaces/exceptions/SocketConfigurationException.hpp" + +namespace communication_interfaces::sockets { + +void ISocket::open() { + this->on_open(); + this->opened_ = true; +} + +bool ISocket::receive_bytes(std::string& buffer) { + if (!this->opened_) { + throw exceptions::SocketConfigurationException("Failed to received bytes: socket has not been opened yet"); + } + return this->on_receive_bytes(buffer); +} + +bool ISocket::send_bytes(const std::string& buffer) { + if (!this->opened_) { + throw exceptions::SocketConfigurationException("Failed to send bytes: socket has not been opened yet"); + } + return this->on_send_bytes(buffer); +} + +void ISocket::close() { + this->opened_ = false; + this->on_close(); +} +}// namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/src/sockets/TCPClient.cpp b/source/communication_interfaces/src/sockets/TCPClient.cpp index a8609d9..7954b64 100644 --- a/source/communication_interfaces/src/sockets/TCPClient.cpp +++ b/source/communication_interfaces/src/sockets/TCPClient.cpp @@ -8,7 +8,7 @@ namespace communication_interfaces::sockets { TCPClient::TCPClient(TCPClientConfiguration configuration) : TCPSocket(configuration.buffer_size), config_(configuration) {} -void TCPClient::open() { +void TCPClient::on_open() { try { bzero((char*) &this->server_address_, sizeof(this->server_address_)); this->server_address_.sin_family = AF_INET; diff --git a/source/communication_interfaces/src/sockets/TCPServer.cpp b/source/communication_interfaces/src/sockets/TCPServer.cpp index 6acdf6d..53455b7 100644 --- a/source/communication_interfaces/src/sockets/TCPServer.cpp +++ b/source/communication_interfaces/src/sockets/TCPServer.cpp @@ -12,10 +12,10 @@ TCPServer::TCPServer(TCPServerConfiguration configuration) : } TCPServer::~TCPServer() { - TCPServer::close(); + TCPServer::on_close(); } -void TCPServer::open() { +void TCPServer::on_open() { try { bzero((char*) &this->server_address_, sizeof(this->server_address_)); this->server_address_.sin_family = AF_INET; @@ -51,8 +51,8 @@ void TCPServer::open() { } } -void TCPServer::close() { +void TCPServer::on_close() { ::close(this->server_fd_); - TCPSocket::close(); + TCPSocket::on_close(); } } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/src/sockets/TCPSocket.cpp b/source/communication_interfaces/src/sockets/TCPSocket.cpp index 6668b0a..52a9049 100644 --- a/source/communication_interfaces/src/sockets/TCPSocket.cpp +++ b/source/communication_interfaces/src/sockets/TCPSocket.cpp @@ -14,10 +14,10 @@ TCPSocket::TCPSocket(int buffer_size) : server_address_(), socket_fd_(), buffer_ } TCPSocket::~TCPSocket() { - TCPSocket::close(); + TCPSocket::on_close(); } -bool TCPSocket::receive_bytes(std::string& buffer) { +bool TCPSocket::on_receive_bytes(std::string& buffer) { std::vector local_buffer(this->buffer_size_); auto receive_length = recv(this->socket_fd_, local_buffer.data(), this->buffer_size_, 0); if (receive_length < 0) { @@ -27,12 +27,12 @@ bool TCPSocket::receive_bytes(std::string& buffer) { return true; } -bool TCPSocket::send_bytes(const std::string& buffer) { +bool TCPSocket::on_send_bytes(const std::string& buffer) { int send_length = send(this->socket_fd_, buffer.data(), buffer.size(), 0); return send_length == static_cast(buffer.size()); } -void TCPSocket::close() { +void TCPSocket::on_close() { ::close(this->socket_fd_); } } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/src/sockets/UDPClient.cpp b/source/communication_interfaces/src/sockets/UDPClient.cpp index 52e3da4..408b8c2 100644 --- a/source/communication_interfaces/src/sockets/UDPClient.cpp +++ b/source/communication_interfaces/src/sockets/UDPClient.cpp @@ -4,15 +4,15 @@ namespace communication_interfaces::sockets { UDPClient::UDPClient(UDPSocketConfiguration configuration) : UDPSocket(std::move(configuration)) {} -void UDPClient::open() { +void UDPClient::on_open() { this->open_socket(false); } -bool UDPClient::receive_bytes(std::string& buffer) { +bool UDPClient::on_receive_bytes(std::string& buffer) { return this->recvfrom(this->server_address_, buffer); } -bool UDPClient::send_bytes(const std::string& buffer) { +bool UDPClient::on_send_bytes(const std::string& buffer) { return this->sendto(this->server_address_, buffer); } } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/src/sockets/UDPServer.cpp b/source/communication_interfaces/src/sockets/UDPServer.cpp index d392b57..d2b9700 100644 --- a/source/communication_interfaces/src/sockets/UDPServer.cpp +++ b/source/communication_interfaces/src/sockets/UDPServer.cpp @@ -4,15 +4,15 @@ namespace communication_interfaces::sockets { UDPServer::UDPServer(UDPSocketConfiguration configuration) : UDPSocket(std::move(configuration)) {} -void UDPServer::open() { +void UDPServer::on_open() { this->open_socket(true); } -bool UDPServer::receive_bytes(std::string& buffer) { +bool UDPServer::on_receive_bytes(std::string& buffer) { return this->recvfrom(this->client_address_, buffer); } -bool UDPServer::send_bytes(const std::string& buffer) { +bool UDPServer::on_send_bytes(const std::string& buffer) { return this->sendto(this->client_address_, buffer); } } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/src/sockets/UDPSocket.cpp b/source/communication_interfaces/src/sockets/UDPSocket.cpp index 81b4485..eea0f90 100644 --- a/source/communication_interfaces/src/sockets/UDPSocket.cpp +++ b/source/communication_interfaces/src/sockets/UDPSocket.cpp @@ -17,7 +17,7 @@ UDPSocket::UDPSocket(UDPSocketConfiguration configuration) : } UDPSocket::~UDPSocket() { - UDPSocket::close(); + UDPSocket::on_close(); } void UDPSocket::open_socket(bool bind_socket) { @@ -75,7 +75,7 @@ bool UDPSocket::sendto(const sockaddr_in& address, const std::string& buffer) co return send_length == static_cast(buffer.size()); } -void UDPSocket::close() { +void UDPSocket::on_close() { if (this->server_fd_ >= 0) { ::close(this->server_fd_); this->server_fd_ = -1; diff --git a/source/communication_interfaces/src/sockets/ZMQPublisher.cpp b/source/communication_interfaces/src/sockets/ZMQPublisher.cpp index 48c9486..15a3728 100644 --- a/source/communication_interfaces/src/sockets/ZMQPublisher.cpp +++ b/source/communication_interfaces/src/sockets/ZMQPublisher.cpp @@ -4,12 +4,12 @@ namespace communication_interfaces::sockets { ZMQPublisher::ZMQPublisher(ZMQSocketConfiguration configuration) : ZMQSocket(std::move(configuration)) {} -void ZMQPublisher::open() { +void ZMQPublisher::on_open() { this->socket_ = std::make_shared(*this->config_.context, ZMQ_PUB); this->open_socket(); } -bool ZMQPublisher::receive_bytes(std::string&) { +bool ZMQPublisher::on_receive_bytes(std::string&) { throw std::runtime_error("Receive not available for socket of type ZMQPublisher"); } } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/src/sockets/ZMQPublisherSubscriber.cpp b/source/communication_interfaces/src/sockets/ZMQPublisherSubscriber.cpp index cdbf5f6..eff31b8 100644 --- a/source/communication_interfaces/src/sockets/ZMQPublisherSubscriber.cpp +++ b/source/communication_interfaces/src/sockets/ZMQPublisherSubscriber.cpp @@ -21,20 +21,20 @@ ZMQPublisherSubscriber::~ZMQPublisherSubscriber() { ZMQPublisherSubscriber::close(); } -void ZMQPublisherSubscriber::open() { +void ZMQPublisherSubscriber::on_open() { this->pub_->open(); this->sub_->open(); } -bool ZMQPublisherSubscriber::receive_bytes(std::string& buffer) { +bool ZMQPublisherSubscriber::on_receive_bytes(std::string& buffer) { return this->sub_->receive_bytes(buffer); } -bool ZMQPublisherSubscriber::send_bytes(const std::string& buffer) { +bool ZMQPublisherSubscriber::on_send_bytes(const std::string& buffer) { return this->pub_->send_bytes(buffer); } -void ZMQPublisherSubscriber::close() { +void ZMQPublisherSubscriber::on_close() { this->pub_->close(); this->sub_->close(); } diff --git a/source/communication_interfaces/src/sockets/ZMQSocket.cpp b/source/communication_interfaces/src/sockets/ZMQSocket.cpp index 85f2c8d..cc70409 100644 --- a/source/communication_interfaces/src/sockets/ZMQSocket.cpp +++ b/source/communication_interfaces/src/sockets/ZMQSocket.cpp @@ -7,7 +7,7 @@ namespace communication_interfaces::sockets { ZMQSocket::ZMQSocket(ZMQSocketConfiguration configuration) : config_(std::move(configuration)) {} ZMQSocket::~ZMQSocket() { - ZMQSocket::close(); + ZMQSocket::on_close(); } void ZMQSocket::open_socket() { @@ -23,7 +23,7 @@ void ZMQSocket::open_socket() { } } -bool ZMQSocket::receive_bytes(std::string& buffer) { +bool ZMQSocket::on_receive_bytes(std::string& buffer) { zmq::recv_flags recv_flag = this->config_.wait ? zmq::recv_flags::none : zmq::recv_flags::dontwait; zmq::message_t message; try { @@ -37,7 +37,7 @@ bool ZMQSocket::receive_bytes(std::string& buffer) { } } -bool ZMQSocket::send_bytes(const std::string& buffer) { +bool ZMQSocket::on_send_bytes(const std::string& buffer) { zmq::send_flags send_flags = this->config_.wait ? zmq::send_flags::none : zmq::send_flags::dontwait; zmq::message_t msg(buffer.size()); memcpy(msg.data(), buffer.data(), buffer.size()); @@ -52,7 +52,7 @@ bool ZMQSocket::send_bytes(const std::string& buffer) { } } -void ZMQSocket::close() { +void ZMQSocket::on_close() { if (this->socket_ != nullptr && this->socket_->connected()) { this->socket_->close(); } diff --git a/source/communication_interfaces/src/sockets/ZMQSubscriber.cpp b/source/communication_interfaces/src/sockets/ZMQSubscriber.cpp index dfce313..5404525 100644 --- a/source/communication_interfaces/src/sockets/ZMQSubscriber.cpp +++ b/source/communication_interfaces/src/sockets/ZMQSubscriber.cpp @@ -4,14 +4,14 @@ namespace communication_interfaces::sockets { ZMQSubscriber::ZMQSubscriber(ZMQSocketConfiguration configuration) : ZMQSocket(std::move(configuration)) {} -void ZMQSubscriber::open() { +void ZMQSubscriber::on_open() { this->socket_ = std::make_shared(*this->config_.context, ZMQ_SUB); this->socket_->set(zmq::sockopt::conflate, 1); this->socket_->set(zmq::sockopt::subscribe, ""); this->open_socket(); } -bool ZMQSubscriber::send_bytes(const std::string&) { +bool ZMQSubscriber::on_send_bytes(const std::string&) { throw std::runtime_error("Send not available for socket of type ZMQSubscriber"); } } // namespace communication_interfaces::sockets diff --git a/source/communication_interfaces/test/tests/test_tcp_communication.cpp b/source/communication_interfaces/test/tests/test_tcp_communication.cpp index b3f21c2..876e199 100644 --- a/source/communication_interfaces/test/tests/test_tcp_communication.cpp +++ b/source/communication_interfaces/test/tests/test_tcp_communication.cpp @@ -51,4 +51,22 @@ TEST_F(TestTCPSockets, TestCommunication) { client.join(); server.join(); + + std::string buffer; + this->server_->close(); + EXPECT_THROW(this->server_->receive_bytes(buffer), exceptions::SocketConfigurationException); + EXPECT_THROW(this->server_->send_bytes(buffer), exceptions::SocketConfigurationException); + this->client_->close(); + EXPECT_THROW(this->client_->receive_bytes(buffer), exceptions::SocketConfigurationException); + EXPECT_THROW(this->client_->send_bytes(buffer), exceptions::SocketConfigurationException); +} + +TEST_F(TestTCPSockets, TestNotOpen) { + std::string buffer; + + EXPECT_THROW(this->server_->receive_bytes(buffer), exceptions::SocketConfigurationException); + EXPECT_THROW(this->server_->send_bytes(buffer), exceptions::SocketConfigurationException); + + EXPECT_THROW(this->client_->receive_bytes(buffer), exceptions::SocketConfigurationException); + EXPECT_THROW(this->client_->send_bytes(buffer), exceptions::SocketConfigurationException); } diff --git a/source/communication_interfaces/test/tests/test_udp_communication.cpp b/source/communication_interfaces/test/tests/test_udp_communication.cpp index 98aabe0..ef332ee 100644 --- a/source/communication_interfaces/test/tests/test_udp_communication.cpp +++ b/source/communication_interfaces/test/tests/test_udp_communication.cpp @@ -39,8 +39,9 @@ TEST_F(TestUDPSockets, SendReceive) { TEST_F(TestUDPSockets, Timeout) { // Create server socket and bind it to a port - this->config_.timeout_duration_sec = 5.0; + this->config_.timeout_duration_sec = 3.0; sockets::UDPServer server(this->config_); + server.open(); // Try to receive a message from client, but expect timeout std::string received_bytes; @@ -58,17 +59,30 @@ TEST_F(TestUDPSockets, PortReuse) { } TEST_F(TestUDPSockets, OpenClose) { + std::string buffer; // Create and open server socket + this->config_.timeout_duration_sec = 0.5; sockets::UDPServer server(this->config_); + EXPECT_THROW(server.send_bytes(buffer), exceptions::SocketConfigurationException); + EXPECT_THROW(server.receive_bytes(buffer), exceptions::SocketConfigurationException); server.open(); + EXPECT_FALSE(server.send_bytes(std::string("test"))); + EXPECT_FALSE(server.receive_bytes(buffer)); // Close server socket server.close(); - - // Create and open client socket + EXPECT_THROW(server.send_bytes(buffer), exceptions::SocketConfigurationException); + EXPECT_THROW(server.receive_bytes(buffer), exceptions::SocketConfigurationException); + sockets::UDPClient client(this->config_); + EXPECT_THROW(client.send_bytes(buffer), exceptions::SocketConfigurationException); + EXPECT_THROW(client.receive_bytes(buffer), exceptions::SocketConfigurationException); client.open(); - // Try to send a message from the closed server socket (expect failure) - EXPECT_FALSE(server.send_bytes(std::string())); + EXPECT_TRUE(client.send_bytes(std::string("test"))); + EXPECT_FALSE(client.receive_bytes(buffer)); + // Close client socket + client.close(); + EXPECT_THROW(client.send_bytes(buffer), exceptions::SocketConfigurationException); + EXPECT_THROW(client.receive_bytes(buffer), exceptions::SocketConfigurationException); } diff --git a/source/communication_interfaces/test/tests/test_zmq_communication.cpp b/source/communication_interfaces/test/tests/test_zmq_communication.cpp index a939689..5c938f6 100644 --- a/source/communication_interfaces/test/tests/test_zmq_communication.cpp +++ b/source/communication_interfaces/test/tests/test_zmq_communication.cpp @@ -68,3 +68,15 @@ TEST_F(TestZMQSockets, SendReceiveCombined) { server.close(); client.close(); } + +TEST_F(TestZMQSockets, TestOpenClose) { + std::string buffer; + sockets::ZMQPublisher socket(this->config_); + EXPECT_THROW(socket.send_bytes(buffer), exceptions::SocketConfigurationException); + EXPECT_THROW(socket.receive_bytes(buffer), std::runtime_error); + + socket.open(); + EXPECT_TRUE(socket.send_bytes(std::string("test"))); + socket.close(); + EXPECT_THROW(socket.send_bytes(buffer), exceptions::SocketConfigurationException); +}