ensuring no duplicate rx hostname port combo (#604)

* rx_hostname and port combo to one, or hostname to all, or a vector of hostnames and ports, ignoring none or empty, then verifying no duplicates for the host port combo including from shared memory

* extracted function for rx_hostname (#694)

* c++14 revert

* unique hostname-port combo for port, hostname, rx_tcpport (#696)

* verify unique combo for rx_port as well

* check unique hostname-port combo also when setting control port, hostname, rx_hostname and rx_tcpport

---------

Co-authored-by: Erik Fröjdh <erik.frojdh@gmail.com>
Co-authored-by: Erik Frojdh <erik.frojdh@psi.ch>

RELEASE.txt

@@ -29,6 +29,8 @@ This document describes the differences between v7.x.x and v7.0.0
 
 
 - moench being made compatible with jungfrau 2.0 boards (jungfrau structure, away from ctb)
+- rx_hostname and port can be combo to one or to all, or vector or hostnames and ports. ignoring none or empty, then verifying no duplicates for the host port combo including from shared memory
+- same for hostname and port combo (for virtual servers)
 - eiger febl and febr in versions, ensure its the same as beb fw version
 - eiger hardware version fx30 and fx70 (versions command)
 - fixed rx_arping error

slsDetectorSoftware/src/Detector.cpp

@@ -1182,7 +1182,8 @@ Result<std::string> Detector::getRxHostname(Positions pos) const {
 }
 
 void Detector::setRxHostname(const std::string &receiver, Positions pos) {
-    pimpl->Parallel(&Module::setReceiverHostname, pos, receiver,
+    auto host = pimpl->verifyUniqueRxHost(receiver, pos);
+    pimpl->Parallel(&Module::setReceiverHostname, pos, host.first, host.second,
                     pimpl->getInitialChecks());
     updateRxRateCorrections();
 }
@@ -1190,17 +1191,15 @@ void Detector::setRxHostname(const std::string &receiver, Positions pos) {
 void Detector::setRxHostname(const std::vector<std::string> &name) {
     // set all to same rx_hostname
     if (name.size() == 1) {
-        pimpl->Parallel(&Module::setReceiverHostname, {}, name[0],
+        auto host = pimpl->verifyUniqueRxHost(name[0], {});
-                        pimpl->getInitialChecks());
+        pimpl->Parallel(&Module::setReceiverHostname, {}, host.first,
+                        host.second, pimpl->getInitialChecks());
     } else {
-        if ((int)name.size() != size()) {
+        auto hosts = pimpl->verifyUniqueRxHost(name);
-            throw RuntimeError(
-                "Receiver hostnames size " + std::to_string(name.size()) +
-                " does not match detector size " + std::to_string(size()));
-        }
         // set each rx_hostname
         for (int idet = 0; idet < size(); ++idet) {
-            pimpl->Parallel(&Module::setReceiverHostname, {idet}, name[idet],
+            pimpl->Parallel(&Module::setReceiverHostname, {idet},
+                            hosts[idet].first, hosts[idet].second,
                             pimpl->getInitialChecks());
         }
     }
@@ -1217,10 +1216,12 @@ void Detector::setRxPort(int port, int module_id) {
         for (auto &it : port_list) {
             it = port++;
         }
+        // no need to verify hostname-port combo as unique port(incremented)
         for (int idet = 0; idet < size(); ++idet) {
             pimpl->Parallel(&Module::setReceiverPort, {idet}, port_list[idet]);
         }
     } else {
+        pimpl->verifyUniqueRxHost(port, module_id);
         pimpl->Parallel(&Module::setReceiverPort, {module_id}, port);
     }
 }
@@ -2460,10 +2461,12 @@ Result<int> Detector::getControlPort(Positions pos) const {
 }
 
 void Detector::setControlPort(int value, Positions pos) {
+    pimpl->verifyUniqueDetHost(value, pos);
     pimpl->Parallel(&Module::setControlPort, pos, value);
 }
 
 Result<int> Detector::getStopPort(Positions pos) const {
+    // not verifying unique stop port (control port is sufficient)
     return pimpl->Parallel(&Module::getStopPort, pos);
 }
 

slsDetectorSoftware/src/DetectorImpl.cpp

@@ -279,31 +279,14 @@ void DetectorImpl::setHostname(const std::vector<std::string> &name) {
     }
 }
 
-void DetectorImpl::addModule(const std::string &hostname) {
+void DetectorImpl::addModule(const std::string &name) {
-    LOG(logINFO) << "Adding module " << hostname;
+    LOG(logINFO) << "Adding module " << name;
-
+    auto host = verifyUniqueDetHost(name);
-    int port = DEFAULT_TCP_CNTRL_PORTNO;
+    std::string hostname = host.first;
-    std::string host = hostname;
+    int port = host.second;
-    auto res = split(hostname, ':');
-    if (res.size() > 1) {
-        host = res[0];
-        port = StringTo<int>(res[1]);
-    }
-
-    if (host != "localhost") {
-        for (auto &module : modules) {
-            if (module->getHostname() == host) {
-                LOG(logWARNING)
-                    << "Module " << host << "already part of the Detector!"
-                    << std::endl
-                    << "Remove it before adding it back in a new position!";
-                return;
-            }
-        }
-    }
 
     // get type by connecting
-    detectorType type = Module::getTypeFromDetector(host, port);
+    detectorType type = Module::getTypeFromDetector(hostname, port);
 
     // gotthard cannot have more than 2 modules (50um=1, 25um=2
     if ((type == GOTTHARD || type == GOTTHARD2) && modules.size() > 2) {
@@ -316,7 +299,7 @@ void DetectorImpl::addModule(const std::string &hostname) {
     shm()->totalNumberOfModules = modules.size();
     modules[pos]->setControlPort(port);
     modules[pos]->setStopPort(port + 1);
-    modules[pos]->setHostname(host, shm()->initialChecks);
+    modules[pos]->setHostname(hostname, shm()->initialChecks);
 
     // module type updated by now
     shm()->detType = Parallel(&Module::getDetectorType, {})
@@ -1538,6 +1521,129 @@ defs::xy DetectorImpl::calculatePosition(int moduleIndex,
     return pos;
 }
 
+void DetectorImpl::verifyUniqueDetHost(const int port,
+                                       std::vector<int> positions) const {
+    // port for given positions
+    if (positions.empty() || (positions.size() == 1 && positions[0] == -1)) {
+        positions.resize(modules.size());
+        std::iota(begin(positions), end(positions), 0);
+    }
+    std::vector<std::pair<std::string, int>> hosts(size());
+    for (auto it : positions) {
+        hosts[it].second = port;
+    }
+    verifyUniqueHost(true, hosts);
+}
+
+void DetectorImpl::verifyUniqueRxHost(const int port,
+                                      const int moduleId) const {
+    std::vector<std::pair<std::string, int>> hosts(size());
+    hosts[moduleId].second = port;
+    verifyUniqueHost(false, hosts);
+}
+
+std::pair<std::string, int>
+DetectorImpl::verifyUniqueDetHost(const std::string &name) {
+    // extract port
+    // C++17 could be auto [hostname, port] = ParseHostPort(name);
+    auto res = ParseHostPort(name);
+    std::string hostname = res.first;
+    int port = res.second;
+    if (port == 0) {
+        port = DEFAULT_TCP_CNTRL_PORTNO;
+    }
+
+    int detSize = size();
+    // mod not yet added
+    std::vector<std::pair<std::string, int>> hosts(detSize + 1);
+    hosts[detSize].first = hostname;
+    hosts[detSize].second = port;
+
+    verifyUniqueHost(true, hosts);
+    return std::make_pair(hostname, port);
+}
+
+std::pair<std::string, int>
+DetectorImpl::verifyUniqueRxHost(const std::string &name,
+                                 std::vector<int> positions) const {
+    // no checks if setting to none
+    if (name == "none" || name.empty()) {
+        return make_pair(name, 0);
+    }
+    // extract port
+    // C++17 could be auto [hostname, port] = ParseHostPort(name);
+    auto res = ParseHostPort(name);
+    std::string hostname = res.first;
+    int port = res.second;
+
+    // hostname and port for given positions
+    if (positions.empty() || (positions.size() == 1 && positions[0] == -1)) {
+        positions.resize(modules.size());
+        std::iota(begin(positions), end(positions), 0);
+    }
+
+    std::vector<std::pair<std::string, int>> hosts(size());
+    for (auto it : positions) {
+        hosts[it].first = hostname;
+        hosts[it].second = port;
+    }
+
+    verifyUniqueHost(false, hosts);
+    return std::make_pair(hostname, port);
+}
+
+std::vector<std::pair<std::string, int>>
+DetectorImpl::verifyUniqueRxHost(const std::vector<std::string> &names) const {
+    if ((int)names.size() != size()) {
+        throw RuntimeError(
+            "Receiver hostnames size " + std::to_string(names.size()) +
+            " does not match detector size " + std::to_string(size()));
+    }
+
+    // extract ports
+    std::vector<std::pair<std::string, int>> hosts;
+    for (const auto &name : names) {
+        hosts.push_back(ParseHostPort(name));
+    }
+
+    verifyUniqueHost(false, hosts);
+    return hosts;
+}
+
+void DetectorImpl::verifyUniqueHost(
+    bool isDet, std::vector<std::pair<std::string, int>> &hosts) const {
+
+    // fill from shm if not provided
+    for (int i = 0; i != size(); ++i) {
+        if (hosts[i].first.empty()) {
+            hosts[i].first = (isDet ? modules[i]->getHostname()
+                                    : modules[i]->getReceiverHostname());
+        }
+        if (hosts[i].second == 0) {
+            hosts[i].second = (isDet ? modules[i]->getControlPort()
+                                     : modules[i]->getReceiverPort());
+        }
+    }
+
+    // remove the ones without a hostname
+    hosts.erase(std::remove_if(hosts.begin(), hosts.end(),
+                               [](const std::pair<std::string, int> &x) {
+                                   return (x.first == "none" ||
+                                           x.first.empty());
+                               }),
+                hosts.end());
+
+    // must be unique
+    if (hasDuplicates(hosts)) {
+        throw RuntimeError(
+            "Cannot set due to duplicate hostname-port number pairs.");
+    }
+
+    for (auto it : hosts) {
+        LOG(logDEBUG) << it.first << " " << it.second << std::endl;
+    }
+}
+
 defs::ROI DetectorImpl::getRxROI() const {
     if (shm()->detType == CHIPTESTBOARD) {
         throw RuntimeError("RxRoi not implemented for this Detector");

slsDetectorSoftware/src/DetectorImpl.h

@@ -300,6 +300,17 @@ class DetectorImpl : public virtual slsDetectorDefs {
                               Positions pos = {});
     void setDefaultDac(defs::dacIndex index, int defaultValue,
                        defs::detectorSettings sett, Positions pos);
+
+    void verifyUniqueDetHost(const int port, std::vector<int> positions) const;
+    void verifyUniqueRxHost(const int port, const int moduleId) const;
+
+    std::pair<std::string, int> verifyUniqueDetHost(const std::string &name);
+    std::pair<std::string, int>
+    verifyUniqueRxHost(const std::string &name,
+                       std::vector<int> positions) const;
+    std::vector<std::pair<std::string, int>>
+    verifyUniqueRxHost(const std::vector<std::string> &names) const;
+
     defs::ROI getRxROI() const;
     void setRxROI(const defs::ROI arg);
     void clearRxROI();
@@ -396,6 +407,10 @@ class DetectorImpl : public virtual slsDetectorDefs {
     defs::xy getPortGeometry() const;
     defs::xy calculatePosition(int moduleIndex, defs::xy geometry) const;
 
+    void
+    verifyUniqueHost(bool isDet,
+                     std::vector<std::pair<std::string, int>> &hosts) const;
+
     const int detectorIndex{0};
     SharedMemory<sharedDetector> shm{0, -1};
     SharedMemory<CtbConfig> ctb_shm{0, -1, CtbConfig::shm_tag()};

slsDetectorSoftware/src/Module.cpp

@@ -1329,30 +1329,33 @@ std::string Module::getReceiverHostname() const {
     return std::string(shm()->rxHostname);
 }
 
-void Module::setReceiverHostname(const std::string &receiverIP,
+void Module::setReceiverHostname(const std::string &hostname, const int port,
                                  const bool initialChecks) {
-    LOG(logDEBUG1) << "Setting up Receiver hostname with " << receiverIP;
+    {
+        std::ostringstream oss;
+        oss << "Setting up Receiver hostname with " << hostname;
+        if (port != 0) {
+            oss << " at port " << port;
+        }
+        LOG(logDEBUG1) << oss.str();
+    }
 
     if (getRunStatus() == RUNNING) {
         throw RuntimeError("Cannot set receiver hostname. Acquisition already "
                            "running. Stop it first.");
     }
 
-    if (receiverIP == "none") {
+    if (hostname == "none") {
         memset(shm()->rxHostname, 0, MAX_STR_LENGTH);
         strcpy_safe(shm()->rxHostname, "none");
         shm()->useReceiverFlag = false;
         return;
     }
 
-    // start updating
+    strcpy_safe(shm()->rxHostname, hostname.c_str());
-    std::string host = receiverIP;
+    if (port != 0) {
-    auto res = split(host, ':');
+        shm()->rxTCPPort = port;
-    if (res.size() > 1) {
-        host = res[0];
-        shm()->rxTCPPort = std::stoi(res[1]);
     }
-    strcpy_safe(shm()->rxHostname, host.c_str());
     shm()->useReceiverFlag = true;
 
     try {

slsDetectorSoftware/src/Module.h

@@ -281,7 +281,7 @@ class Module : public virtual slsDetectorDefs {
      * ************************************************/
     bool getUseReceiverFlag() const;
     std::string getReceiverHostname() const;
-    void setReceiverHostname(const std::string &receiver,
+    void setReceiverHostname(const std::string &hostname, const int port,
                              const bool initialChecks);
     int getReceiverPort() const;
     int setReceiverPort(int port_number);

slsSupportLib/include/sls/container_utils.h

@@ -10,6 +10,7 @@
 #include <string>
 #include <type_traits>
 #include <vector>
+// #include <utility> //support pair in vectors
 
 #include "sls/TypeTraits.h"
 
@@ -148,6 +149,12 @@ Squash(const Container &c, typename Container::value_type default_value = {}) {
     return default_value;
 }
 
+template <typename Container> bool hasDuplicates(Container c) {
+    std::sort(c.begin(), c.end());
+    auto pos = std::adjacent_find(c.begin(), c.end());
+    return pos != c.end(); // if we found something there are duplicates
+}
+
 template <typename T>
 typename std::enable_if<is_container<T>::value, bool>::type
 removeDuplicates(T &c) {

slsSupportLib/include/sls/string_utils.h

@@ -5,6 +5,7 @@
 #include <cassert>
 #include <cstring>
 #include <string>
+#include <utility>
 #include <vector>
 
 namespace sls {
@@ -59,4 +60,6 @@ bool is_int(const std::string &s);
 bool replace_first(std::string *s, const std::string &substr,
                    const std::string &repl);
 
+std::pair<std::string, int> ParseHostPort(const std::string &s);
+
 } // namespace sls

slsSupportLib/src/string_utils.cpp

@@ -50,4 +50,17 @@ bool replace_first(std::string *s, const std::string &substr,
     return false;
 }
 
+std::pair<std::string, int> ParseHostPort(const std::string &s) {
+    // TODO deal with to many :, port not there?
+    // no port return hostname as is and port as 0
+    std::string host;
+    int port{0};
+    auto res = split(s, ':');
+    host = res[0];
+    if (res.size() > 1) {
+        port = std::stoi(res[1]);
+    }
+    return std::make_pair(host, port);
+}
+
 }; // namespace sls

slsSupportLib/tests/test-container_utils.cpp

@@ -136,6 +136,23 @@ TEST_CASE("compare a vector of arrays", "[support]") {
     CHECK(minusOneIfDifferent(vec1) == arr);
 }
 
+TEST_CASE("check if vector has duplicates") {
+    std::vector<int> vec{1, 0, 2, 5, 3, 1, 8, 6};
+    REQUIRE(hasDuplicates(vec) == true);
+}
+
+TEST_CASE("check for duplicates in vector of pairs") {
+    std::vector<std::pair<std::string, int>> vec;
+    vec.emplace_back("localhost", 1954);
+    REQUIRE(hasDuplicates(vec) == false);
+
+    vec.emplace_back("localhost", 1800);
+    REQUIRE(hasDuplicates(vec) == false);
+
+    vec.emplace_back("localhost", 1954);
+    REQUIRE(hasDuplicates(vec) == true);
+}
+
 TEST_CASE("remove duplicates from vector") {
     std::vector<int> v{5, 6, 5, 3};
     auto r = removeDuplicates(v);

slsSupportLib/tests/test-string_utils.cpp

@@ -108,6 +108,21 @@ TEST_CASE("replace --help") {
     REQUIRE(s == "list");
 }
 
+TEST_CASE("port host") {
+    std::string hostport = "localhost:1954";
+    auto res = ParseHostPort(hostport);
+    REQUIRE(res.first == "localhost");
+    REQUIRE(res.second == 1954);
+}
+
+TEST_CASE("port missing") {
+    // TODO! is this the intended result?
+    std::string host = "localhost";
+    auto res = ParseHostPort(host);
+    REQUIRE(res.first == "localhost");
+    REQUIRE(res.second == 0);
+}
+
 // TEST_CASE("concat things not being strings")
 
 } // namespace sls