Updated documentation and solved TODOs

ecal/service/ecal_service/include/ecal/service/client_manager.h

@@ -147,8 +147,7 @@ namespace eCAL
        * stopped from this central place.
        * 
        * @param protocol_version  The protocol version to use for the client session. If 0, the legacy buggy protocol will be used.
-       * @param address           The address of the server to connect to TODO: Update documentation
-       * @param port              The port of the server to connect to
+       * @param server_list       A list of endpoints to connect to. Must not be empty. The endpoints will be tried in the given order until a working endpoint is found.
        * @param event_callback    The callback, that will be called, when the client has connected to the server or disconnected from it. The callback will be executed in the io_context thread.
        * 
        * @return A shared_ptr to the newly created ClientSession instance

ecal/service/ecal_service/include/ecal/service/client_session.h

@@ -136,8 +136,7 @@ namespace eCAL
        * 
        * @param io_context        The io_context to use for the session and all callbacks.
        * @param protocol_version  The protocol version to use for the session. When this is 0, the legacy buggy protocol is used.
-       * @param address           The address of the server to connect to. May be an IP or a Hostname, IPv6 is supported. TODO: Update documentation
-       * @param port              The port of the server to connect to.
+       * @param server_list       A list of endpoints to connect to. Must not be empty. The endpoints will be tried in the given order until a working endpoint is found.
        * @param event_callback    The callback to be called when the session's state changes, i.e. when the session successfully connected to a server or disconnected from it.
        * @param logger            The logger to use for logging.
        * @param delete_callback   The callback to be called when the session is deleted. This is useful for the eCAL::service::ClientManager to keep track of the number of active sessions.
@@ -225,30 +224,40 @@ namespace eCAL
        */
       eCAL::service::Error call_service(const std::shared_ptr<const std::string>& request, std::shared_ptr<std::string>& response);
 
-      /** TODO: Revise documentation
-       * @brief Get the address that this client session has been created with.
+      /**
+       * @brief Get the host that this client is connected to.
+       * 
+       * Get the host that this client is connected to.
+       * If the client is not connected, this function will return an empty
+       * string. Otherwise, it will return the hostname from the list
+       * server_list that the client is connected to.
        * 
-       * This function returns the address that this client session has been
-       * created with. It will not return the address of the server that this
-       * client session is connected to, which would actually be the same
-       * address, but probably resolved to an IP.
+       * The host is not resolved to an IP address. Use get_remote_endpoint()
+       * to get the actual IP address.
        * 
-       * @return The address that this client session has been created with.
+       * @return The host that this client is connected to.
        */
       std::string get_host() const;
 
-      /**TODO: Revise documentation
-       * @brief Get the port that this client session has been created with.
+      /**
+       * @brief Get the port that this client session is connected to.
        * 
-       * This function returns the port that this client session has been
-       * created with. It is not said, that the connection has been established
-       * successfully.
+       * Get the port that this client session is connected to. If the client
+       * is not connected, this function will return 0. Otherwise, it will
+       * return the port from the list server_list that the client is connected
+       * to.
        * 
-       * @return The port that this client session has been created with.
+       * @return The port that this client is connected to
        */
       std::uint16_t get_port()                     const;
 
-      // TODO: Document
+      /**
+       * @brief Get the remote endpoint that this client session is connected to.
+       * 
+       * Get the remote endpoint that this client session is connected to. Only
+       * valid, if the client session is actually connected to a server. If a
+       * hostname was given, this function will return the resolved IP address.
+       */
       asio::ip::tcp::endpoint get_remote_endpoint() const;
 
       /**

ecal/service/ecal_service/src/client_session_impl_base.h

@@ -75,7 +75,7 @@ namespace eCAL
 
       virtual std::string             get_host()            const = 0;
       virtual std::uint16_t           get_port()            const = 0;
-      virtual asio::ip::tcp::endpoint get_remote_endpoint() const = 0; // TODO: Document
+      virtual asio::ip::tcp::endpoint get_remote_endpoint() const = 0;
 
       virtual State         get_state()                     const = 0;
       virtual std::uint8_t  get_accepted_protocol_version() const = 0;

ecal/service/ecal_service/src/client_session_impl_v0.cpp

@@ -34,6 +34,7 @@
 #include <cstdint>
 #include <memory>
 #include <mutex>
+#include <stdexcept>
 #include <string>
 #include <utility>
 #include <vector>
@@ -54,7 +55,13 @@ namespace eCAL
     {
       std::shared_ptr<ClientSessionV0> instance(new ClientSessionV0(io_context, server_list, event_callback, logger));
 
-      instance->resolve_endpoint(0); // TODO: Write a test that checks what happens when the server_list is empty
+      // Throw exception, if the server list is empty
+      if (server_list.empty())
+      {
+        throw std::invalid_argument("Server list must not be empty");
+      }
+
+      instance->resolve_endpoint(0);
 
       return instance;
     }

ecal/service/ecal_service/src/client_session_impl_v0.h

@@ -101,7 +101,7 @@ namespace eCAL
     public:
       std::string             get_host()            const override;
       std::uint16_t           get_port()            const override;
-      asio::ip::tcp::endpoint get_remote_endpoint() const override; // TODO: Document
+      asio::ip::tcp::endpoint get_remote_endpoint() const override;
 
       State         get_state()                     const override;
       std::uint8_t  get_accepted_protocol_version() const override;

ecal/service/ecal_service/src/client_session_impl_v1.cpp

@@ -29,6 +29,7 @@
 #include <cstdint>
 #include <memory>
 #include <mutex>
+#include <stdexcept>
 #include <string>
 #include <utility>
 #include <vector>
@@ -63,6 +64,12 @@ namespace eCAL
     {
       std::shared_ptr<ClientSessionV1> instance(new ClientSessionV1(io_context, server_list, event_callback, logger));
 
+      // Throw exception, if the server list is empty
+      if (server_list.empty())
+      {
+        throw std::invalid_argument("Server list must not be empty");
+      }
+
       instance->resolve_endpoint(0);
 
       return instance;

ecal/service/ecal_service/src/client_session_impl_v1.h

@@ -105,7 +105,7 @@ namespace eCAL
     public:
       std::string             get_host()            const override;
       std::uint16_t           get_port()            const override;
-      asio::ip::tcp::endpoint get_remote_endpoint() const override; // TODO: Document
+      asio::ip::tcp::endpoint get_remote_endpoint() const override;
 
       State         get_state()                     const override;
       std::uint8_t  get_accepted_protocol_version() const override;

ecal/service/test/src/ecal_tcp_service_test.cpp

@@ -26,6 +26,7 @@
 #include <iostream>
 #include <string>
 #include <thread>
+#include <stdexcept>
 
 #include <ecal/service/server.h> // Should not be needed, when I use the server manager / client manager
 #include <ecal/service/client_session.h> // Should not be needed, when I use the server manager / client manager
@@ -59,8 +60,6 @@ eCAL::service::LoggerT critical_logger(const std::string& node_name)
 constexpr std::uint8_t min_protocol_version = 0;
 constexpr std::uint8_t max_protocol_version = 1;
 
-// TODO: Add test for the new multi-host feature, where the second host is tried if the connection to the first one fails
-
 #if 1
 TEST(ecal_service, RAII_TcpServiceServer) // NOLINT
 {
@@ -1851,6 +1850,27 @@ TEST(ecal_service, BackupHost)
 }
 #endif
 
+#if 1
+// Test that the client create throws an exception when bein created with an empty server list
+TEST(ecal_service, EmptyServerList)
+{
+  // Regular client
+  for (std::uint8_t protocol_version = min_protocol_version; protocol_version <= max_protocol_version; protocol_version++)
+  {
+    const auto io_context = std::make_shared<asio::io_context>();    
+    EXPECT_THROW(eCAL::service::ClientSession::create(io_context, protocol_version, {}, [](eCAL::service::ClientEventType, const std::string&) -> void {}), std::invalid_argument);
+  }
+
+  // Client manager
+  for (std::uint8_t protocol_version = min_protocol_version; protocol_version <= max_protocol_version; protocol_version++)
+  {
+    const auto io_context = std::make_shared<asio::io_context>();
+    auto client_manager = eCAL::service::ClientManager::create(io_context);
+    EXPECT_THROW(client_manager->create_client(protocol_version, {}, [](eCAL::service::ClientEventType, const std::string&) -> void {}), std::invalid_argument);
+  }
+}
+#endif
+
 #if 1
 TEST(ecal_service, ErrorCallback_ErrorCallbackNoServer) // NOLINT
 {