Code Review 3

slsDetectorServers/slsDetectorServer_cpp/include/DetectorServer.hpp

@@ -38,12 +38,12 @@ template <typename DerivedDetectorServer> class DetectorServer {
 
     std::unique_ptr<DetectorServerImpl> impl;
 
-    auto *getDerivedImpl() {
+    auto *getImpl() {
         return static_cast<typename DerivedDetectorServer::ImplType *>(
             impl.get());
     }
 
-    const auto *getDerivedImpl() const {
+    const auto *getImpl() const {
         return static_cast<const typename DerivedDetectorServer::ImplType *>(
             impl.get());
     }
@@ -191,8 +191,7 @@ ProcessedResult DetectorServer<DerivedDetectorServer>::get_update_mode(
 
     // TODO: catch the socket error during Send and add error message to the
     // ProcessedResult but DatSocket shared with receiver - some refactoring
-    return ProcessedResult{static_cast<ReturnCode>(
-        socket.sendResult(static_cast<int>(updateMode)))};
+    return send_result(socket, static_cast<uint32_t>(updateMode));
 }
 
 template <typename DerivedDetectorServer>
@@ -200,8 +199,7 @@ ProcessedResult DetectorServer<DerivedDetectorServer>::get_source_udp_mac(
     ServerInterface &socket) const {
     auto srcUdpMac = impl->get_source_udp_mac();
 
-    return ProcessedResult{
-        static_cast<ReturnCode>(socket.sendResult(srcUdpMac))};
+    return send_result(socket, srcUdpMac);
 }
 
 template <typename DerivedDetectorServer>
@@ -220,7 +218,7 @@ ProcessedResult DetectorServer<DerivedDetectorServer>::set_source_udp_mac(
     }
 
     try {
-        getDerivedImpl()->set_source_udp_mac(newsrcudpMac);
+        getImpl()->set_source_udp_mac(newsrcudpMac);
     } catch (const std::exception &e) {
         LOG(logERROR) << "Failed to set source UDP MAC address: " << e.what();
         return_fail("Failed to set source UDP MAC address: " +
@@ -254,8 +252,7 @@ template <typename DerivedDetectorServer>
 ProcessedResult DetectorServer<DerivedDetectorServer>::get_source_udp_ip(
     ServerInterface &socket) const {
     uint32_t src_UdpIp = impl->get_source_udp_ip();
-    return ProcessedResult{
-        static_cast<ReturnCode>(socket.sendResult(src_UdpIp))};
+    return send_result(socket, src_UdpIp);
 }
 
 template <typename DerivedDetectorServer>
@@ -283,8 +280,7 @@ ProcessedResult DetectorServer<DerivedDetectorServer>::get_destination_udp_mac(
     ServerInterface &socket) const {
 
     auto dstUdpMac = impl->get_destination_udp_mac();
-    return ProcessedResult{
-        static_cast<ReturnCode>(socket.sendResult(dstUdpMac))};
+    return send_result(socket, dstUdpMac);
 }
 
 template <typename DerivedDetectorServer>
@@ -312,8 +308,7 @@ ProcessedResult DetectorServer<DerivedDetectorServer>::get_destination_udp_ip(
     ServerInterface &socket) const {
 
     uint32_t dstUdpIp = impl->get_destination_udp_ip();
-    return ProcessedResult{
-        static_cast<ReturnCode>(socket.sendResult(dstUdpIp))};
+    return send_result(socket, dstUdpIp);
 }
 
 template <typename DerivedDetectorServer>
@@ -340,8 +335,7 @@ template <typename DerivedDetectorServer>
 ProcessedResult DetectorServer<DerivedDetectorServer>::get_destination_udp_port(
     ServerInterface &socket) const {
     uint16_t dstUdpPort = impl->get_destination_udp_port();
-    return ProcessedResult{
-        static_cast<ReturnCode>(socket.sendResult(dstUdpPort))};
+    return send_result(socket, dstUdpPort);
 };
 
 template <typename DerivedDetectorServer>
@@ -349,15 +343,14 @@ ProcessedResult DetectorServer<DerivedDetectorServer>::get_num_frames(
     ServerInterface &socket) const {
     uint64_t num_frames{};
     try {
-        num_frames = getDerivedImpl()->get_num_frames();
+        num_frames = getImpl()->get_num_frames();
     } catch (const std::exception &e) {
         auto error_message =
             "Failed to get number of frames: " + std::string(e.what());
         LOG(logERROR) << error_message;
         return return_fail(error_message);
     }
-    return ProcessedResult{
-        static_cast<ReturnCode>(socket.sendResult(num_frames))};
+    return send_result(socket, num_frames);
 }
 
 template <typename DerivedDetectorServer>
@@ -373,7 +366,7 @@ DetectorServer<DerivedDetectorServer>::set_num_frames(ServerInterface &socket) {
         return return_fail(error_message);
     }
     try {
-        getDerivedImpl()->set_num_frames(num_frames);
+        getImpl()->set_num_frames(num_frames);
     } catch (const std::exception &e) {
         auto error_message =
             "Failed to set number of frames: " + std::string(e.what());
@@ -388,16 +381,14 @@ ProcessedResult DetectorServer<DerivedDetectorServer>::get_num_triggers(
     ServerInterface &socket) const {
     uint64_t num_triggers{};
     try {
-        num_triggers =
-            static_cast<uint64_t>(getDerivedImpl()->get_num_triggers());
+        num_triggers = static_cast<uint64_t>(getImpl()->get_num_triggers());
     } catch (const std::exception &e) {
         auto error_message =
             "Failed to get number of triggers: " + std::string(e.what());
         LOG(logERROR) << error_message;
         return return_fail(error_message);
     }
-    return ProcessedResult{
-        static_cast<ReturnCode>(socket.sendResult(num_triggers))};
+    return send_result(socket, num_triggers);
 }
 
 template <typename DerivedDetectorServer>
@@ -413,7 +404,7 @@ ProcessedResult DetectorServer<DerivedDetectorServer>::set_num_triggers(
         return return_fail(error_message);
     }
     try {
-        getDerivedImpl()->set_num_triggers(num_triggers);
+        getImpl()->set_num_triggers(num_triggers);
     } catch (const std::exception &e) {
         auto error_message =
             "Failed to set number of triggers: " + std::string(e.what());
@@ -428,32 +419,30 @@ ProcessedResult DetectorServer<DerivedDetectorServer>::get_version(
     ServerInterface &socket) const {
 
     auto version =
-        getDerivedImpl()
-            ->get_server_version(); // TODO: get Impl from derived server
+        getImpl()->get_server_version(); // TODO: get Impl from derived server
 
     char version_cstr[MAX_STR_LENGTH]{};
     std::snprintf(version_cstr, sizeof(version_cstr), "%s",
                   version.c_str()); // ensures temination
     LOG(TLogLevel::logDEBUG) << "Server Version: " << version;
-    return ProcessedResult{static_cast<ReturnCode>(socket.sendResult(
-        version_cstr))}; // TODO: check what would be possible return codes!!!
+    return send_result(
+        socket,
+        version_cstr); // TODO: check what would be possible return codes!!!
 }
 
 template <typename DerivedDetectorServer>
 ProcessedResult DetectorServer<DerivedDetectorServer>::get_num_udp_interfaces(
     ServerInterface &socket) const {
-    auto num_udp_interfaces = getDerivedImpl()->get_num_udp_interfaces();
+    int num_udp_interfaces = getImpl()->get_num_udp_interfaces();
 
-    return ProcessedResult{static_cast<ReturnCode>(
-        socket.sendResult(static_cast<int>(num_udp_interfaces)))};
+    return send_result(socket, num_udp_interfaces);
 }
 
 template <typename DerivedDetectorServer>
 ProcessedResult DetectorServer<DerivedDetectorServer>::get_detector_type(
     ServerInterface &socket) const {
-    uint32_t detectortype = getDerivedImpl()->get_detector_type();
-    return ProcessedResult{
-        static_cast<ReturnCode>(socket.sendResult(detectortype))};
+    uint32_t detectortype = getImpl()->get_detector_type();
+    return send_result(socket, detectortype);
 }
 
 template <typename DerivedDetectorServer>
@@ -461,39 +450,37 @@ ProcessedResult DetectorServer<DerivedDetectorServer>::get_receiver_parameters(
     ServerInterface &socket) const {
 
     slsDetectorDefs::rxParameters rx_params =
-        getDerivedImpl()->get_receiver_parameters();
+        getImpl()->get_receiver_parameters();
 
-    return ProcessedResult{
-        static_cast<ReturnCode>(socket.sendResult(rx_params))};
+    return send_result(socket, rx_params);
 }
 
 template <typename DerivedDetectorServer>
 ProcessedResult DetectorServer<DerivedDetectorServer>::get_run_status(
     ServerInterface &socket) const {
 
-    slsDetectorDefs::runStatus status = getDerivedImpl()->get_run_status();
+    slsDetectorDefs::runStatus status = getImpl()->get_run_status();
 
-    return ProcessedResult{static_cast<ReturnCode>(socket.sendResult(status))};
+    return send_result(socket, status);
 }
 
 template <typename DerivedDetectorServer>
 ProcessedResult DetectorServer<DerivedDetectorServer>::initial_checks(
     ServerInterface &socket) const {
-    auto detectorsetupstatus = getDerivedImpl()->initial_checks();
+    auto detectorsetupstatus = getImpl()->get_detector_setup_status();
 
     // TODO: should there be a time limit?
     while (detectorsetupstatus.setup_status ==
            detector_setup_status::NOT_SETUP) {
         std::this_thread::sleep_for(std::chrono::seconds(1));
-        detectorsetupstatus = getDerivedImpl()->initial_checks();
+        detectorsetupstatus = getImpl()->get_detector_setup_status();
     }
     if (detectorsetupstatus.setup_status ==
         detector_setup_status::FAILED_SETUP) {
         return return_fail("Initial checks failed: " +
                            detectorsetupstatus.error_message);
     } else {
-        return ProcessedResult{
-            static_cast<ReturnCode>(socket.sendResult(true))};
+        return send_result<bool>(socket, true);
     }
 }
 
@@ -516,8 +503,7 @@ DetectorServer<DerivedDetectorServer>::set_module_position_and_update_srcudpmac(
     }
 
     try {
-        getDerivedImpl()->set_module_position_and_update_srcudpmac(
-            position_info);
+        getImpl()->set_module_position_and_update_srcudpmac(position_info);
     } catch (const std::exception &e) {
         return_fail("Failed to set module position: " + std::string(e.what()));
     }

slsDetectorServers/slsDetectorServer_cpp/include/DetectorServerImpl.hpp

@@ -62,12 +62,12 @@ class DetectorServerImpl {
 
     void set_source_udp_ip(const uint32_t srcip);
 
+    uint32_t get_source_udp_ip() const;
+
     void set_destination_udp_ip(const uint32_t dstip);
 
     uint32_t get_destination_udp_ip() const;
 
-    uint32_t get_source_udp_ip() const;
-
     void set_destination_udp_mac(const uint64_t dstmac);
 
     uint64_t get_destination_udp_mac() const;
@@ -76,7 +76,7 @@ class DetectorServerImpl {
 
     uint16_t get_destination_udp_port() const;
 
-    detector_setup_status initial_checks() const;
+    detector_setup_status get_detector_setup_status() const;
 
   protected:
     std::array<UDPInfo, 1>

slsDetectorServers/slsDetectorServer_cpp/include/TCPInterface.hpp

@@ -30,6 +30,11 @@ inline ProcessedResult send_ok(ServerInterface &socket) {
         static_cast<ReturnCode>(socket.Send(ReturnCode::OK))};
 }
 
+template <typename T>
+inline ProcessedResult send_result(ServerInterface &socket, const T &value) {
+    return ProcessedResult{static_cast<ReturnCode>(socket.sendResult(value))};
+}
+
 /**
  * @brief TCPInterface class handles communication and processing of commands
  * from Client to Server.

slsDetectorServers/slsDetectorServer_cpp/include/helpers/Helpers.hpp

@@ -1,4 +1,7 @@
 #pragma once
+#include "Defs.hpp"
+#include "DetectorServerImpl.hpp"
+#include "sls/SharedMemory.h"
 #include "sls/sls_detector_defs.h"
 #include <cstdint>
 #include <cstdlib>
@@ -6,13 +9,13 @@
 namespace sls {
 
 constexpr uint64_t mac_mask = 0xffffffffffff0000;
-constexpr uint8_t offset_row_position_in_mac = 16; // given in bits
-constexpr uint8_t offset_col_position_in_mac = 0;  // given in bits
+constexpr uint8_t offset_row_position_in_mac = 8; // given in bits
+constexpr uint8_t offset_col_position_in_mac = 0; // given in bits
 
 /// @brief generates a random locally administered unicast MAC address for the
 /// source UDP
 /// @return generated MAC address
-inline uint64_t generaterandomMacAddress() {
+inline uint64_t generateRandomMacAddress() {
     uint64_t mac =
         0xAA0000000000; // locally administered unicast address (0xA: 0b1010) //
                         // TODO maybe 0x02000000000 better?
@@ -28,11 +31,10 @@ inline uint64_t generaterandomMacAddress() {
 /// @param module_row
 /// @param module_col
 /// @return generated MAC address
-inline uint64_t
-generate_mac_address_from_module_position(const size_t module_row,
-                                          const size_t module_col) {
+inline uint64_t generateMacAddressfromModulePosition(const uint8_t module_row,
+                                                     const uint8_t module_col) {
 
-    uint64_t newSrcMac = generaterandomMacAddress();
+    uint64_t newSrcMac = generateRandomMacAddress();
     newSrcMac = (newSrcMac & mac_mask) |
                 (module_row << offset_row_position_in_mac) |
                 (module_col << offset_col_position_in_mac);
@@ -40,4 +42,23 @@ generate_mac_address_from_module_position(const size_t module_row,
     return newSrcMac;
 }
 
-} // namespace sls
+/// @brief check that mac is unicast and locally administered
+/// @param mac
+/// @return true if mac is valid, false otherwise
+inline bool isValidMac(const uint64_t mac) {
+    if ((mac << INDIVIDUAL_GROUP_BIT_OFFSET) == 0 &&
+        (mac << UNIVERSAL_LOCAL_BIT_OFFSET) == 1) {
+        return true;
+    }
+    return false;
+}
+
+inline void freeSharedMemory() {
+    SharedMemory<acquisitionStatus> shm(0, -1, "server");
+
+    if (shm.exists()) {
+        shm.removeSharedMemory();
+    }
+}
+
+} // namespace sls