clean code for thread for arping

slsReceiverSoftware/CMakeLists.txt

@@ -12,7 +12,7 @@ set(SOURCES
     src/DataProcessor.cpp
     src/DataStreamer.cpp
     src/Fifo.cpp
-    src/ThreadArping.cpp
+    src/Arping.cpp
 )
 
 set(PUBLICHEADERS

slsReceiverSoftware/src/Arping.cpp

@@ -3,61 +3,98 @@
 
 #include "Arping.h"
 
-#include <iostream>
+#include <chrono>
+#include <sys/syscall.h>
+#include <unistd.h>
 
-const std::string Arping::ThreadType = "Arping";
+void Arping::ClearIpsAndInterfaces() { commands.clear(); }
 
-Arping::Arping(nt ind) : ThreadObject(ind, ThreadType) {}
-
-Arping::~Arping() = default;
-
-void Arping::ClearIpsAndInterfaces() {
-    arpInterfaceIp.clear();
-    commands.clear();
-}
-
-void Arping::AddInterfacesAndIps(std::string interface, std::string ip) {
+void Arping::AddInterfacesAndIps(const std::string &interface,
+                                 const std::string &ip) {
     // create commands to arping
     std::ostringstream os;
     os << "arping -c 1 -U -I " << interface << " " << ip;
     // to read error messages
     os << " 2>&1";
     std::string cmd = os.str();
-    arpingCommands.push_back(cmd);
+    commands.push_back(cmd);
+}
+
+pid_t Arping::GetThreadId() const { return threadId; }
+
+bool Arping::IsRunning() const { return runningFlag; }
+
+void Arping::StartThread() {
+    TestCommands();
+    try {
+        t = std::thread(&Arping::ThreadExecution, this);
+    } catch (...) {
+        throw sls::RuntimeError("Could not start arping thread");
+    }
+    runningFlag = true;
+}
+
+void Arping::StopThread() {
+    runningFlag = false;
+    t.join();
 }
 
 void Arping::ThreadExecution() {
-    // arping
+    threadId = syscall(SYS_gettid);
+    LOG(logINFOBLUE) << "Created [ Arping Thread, Tid: " << threadId << "]";
 
-    // wait for 60s
-    usleep(60 * 1000 * 1000);
+    while (runningFlag) {
+        std::string error = ExecuteCommands();
+        // just print (was already tested at thread start)
+        if (!error.empty()) {
+            LOG(logERROR) << error;
+        }
+
+        // wait for 60s as long as thread not killed
+        int nsecs = 0;
+        while (runningFlag && nsecs != 60) {
+            std::this_thread::sleep_for(std::chrono::seconds(1));
+            ++nsecs;
+        }
+    }
+
+    LOG(logINFOBLUE) << "Exiting [ Arping Thread, Tid: " << threadId << " ]";
+    threadId = 0;
 }
 
-LOG(logINFOBLUE) << "Exiting [ Arping Thread, Tid: " << threadId << " ]";
+void Arping::TestCommands() {
+    std::string error = ExecuteCommands();
+    if (!error.empty()) {
+        throw sls::RuntimeError(error);
+    }
 }
 
-void Arping::ExecuteCommands() {
+std::string Arping::ExecuteCommands() {
     for (auto cmd : commands) {
         LOG(logDEBUG) << "Executing Arping Command: " << cmd;
 
         // execute command
         FILE *sysFile = popen(cmd.c_str(), "r");
         if (sysFile == NULL) {
-            LOG(logERROR) << "Executing cmd [" cmd << " ] Fail:"
-                          << "\n\t Popen fail";
-            continue;
+            std::ostringstream os;
+            os << "Could not Arping [" << cmd << " ] : Popen fail";
+            return os.str();
         }
 
-        // check for errors
+        // copy output
         char output[MAX_STR_LENGTH] = {0};
         fgets(output, sizeof(output), sysFile);
         output[sizeof(output) - 1] = '\0';
 
+        // check exit status of command
         if (pclose(sysFile)) {
-            LOG(logERROR) << "Executing cmd[" << cmd
-                          << "]\n\tError Message : " << output;
+            std::ostringstream os;
+            os << "Could not arping[" << cmd << "] : " << output;
+            return os.str();
         } else {
             LOG(logDEBUG) << output;
         }
     }
+
+    return std::string();
 }

slsReceiverSoftware/src/Arping.h

@@ -6,24 +6,30 @@
 is listening to.
  */
 
-#include "ThreadObject.h"
+#include "sls/logger.h"
+#include "sls/sls_detector_defs.h"
 
-class Arping : private virtual slsDetectorDefs, public ThreadObject {
+#include <atomic>
+#include <thread>
+
+class Arping : private virtual slsDetectorDefs {
 
   public:
-    Arping(int ind);
-    ~Arping();
     void ClearIpsAndInterfaces();
-    void AddInterfacesAndIps(std::string interface, std::string ip);
+    void AddInterfacesAndIps(const std::string &interface,
+                             const std::string &ip);
+    pid_t GetThreadId() const;
+    bool IsRunning() const;
+    void StartThread();
+    void StopThread();
 
   private:
-    /**
-     * Thread Execution for Arping Class
-     * Arping interfaces and wait 60 seconds
-     */
-    void ThreadExecution() override;
-    void ExecuteCommands();
+    void TestCommands();
+    std::string ExecuteCommands();
+    void ThreadExecution();
 
-    static const std::string ThreadType;
-    std::vector<std::string> arpingCommands;
+    std::vector<std::string> commands;
+    std::atomic<bool> runningFlag{false};
+    std::thread t;
+    pid_t threadId{0};
 };

slsReceiverSoftware/src/Implementation.cpp

@@ -1,13 +1,13 @@
 // SPDX-License-Identifier: LGPL-3.0-or-other
 // Copyright (C) 2021 Contributors to the SLS Detector Package
 #include "Implementation.h"
+#include "Arping.h"
 #include "DataProcessor.h"
 #include "DataStreamer.h"
 #include "Fifo.h"
 #include "GeneralData.h"
 #include "Listener.h"
 #include "MasterAttributes.h"
-#include "ThreadArping.h"
 #include "sls/ToString.h"
 #include "sls/ZmqSocket.h" //just for the zmq port define
 #include "sls/file_utils.h"
@@ -109,8 +109,8 @@ void Implementation::SetupFifoStructure() {
 
 void Implementation::setDetectorType(const detectorType d) {
 
-    // object to create threads to arping
-    threadArping = sls::make_unique<ThreadArping>();
+    // object to create thread for arping
+    arping = sls::make_unique<Arping>();
 
     detType = d;
     switch (detType) {
@@ -325,30 +325,28 @@ std::array<pid_t, NUM_RX_THREAD_IDS> Implementation::getThreadIds() const {
             retval[id++] = 0;
         }
     }
-    if (threadArping->IsRunning()) {
-        retval[NUM_RX_THREAD_IDS - 1] = threadArping->GetThreadId();
-    }
+    retval[NUM_RX_THREAD_IDS - 1] = arping->GetThreadId();
     return retval;
 }
 
-bool Implementation::getArping() const { return threadArping->IsRunning(); }
+bool Implementation::getArping() const { return arping->IsRunning(); }
 
 pid_t Implementation::getArpingThreadId() const {
-    return threadArping->GetThreadId();
+    return arping->GetThreadId();
 }
 
 void Implementation::setArping(const bool i,
                                const std::vector<std::string> ips) {
-    if (i != threadArping->IsRunning()) {
+    if (i != arping->IsRunning()) {
         if (!i) {
-            threadArping->StopRunning();
+            arping->StopThread();
         } else {
-            threadArping->ClearIpsAndInterfaces();
-            threadArping->AddInterfacesAndIps(eth[0], ips[0]);
+            arping->ClearIpsAndInterfaces();
+            arping->AddInterfacesAndIps(eth[0], ips[0]);
             if (numUDPInterfaces == 2 && detType != EIGER) {
-                threadArping->AddInterfacesAndIps(eth[1], ips[1]);
+                arping->AddInterfacesAndIps(eth[1], ips[1]);
             }
-            threadArping->StartRunning();
+            arping->StartThread();
         }
     }
 }

slsReceiverSoftware/src/Implementation.h

@@ -11,7 +11,7 @@ class DataProcessor;
 class DataStreamer;
 class Fifo;
 class slsDetectorDefs;
-class ThreadArping;
+class Arping;
 
 #include <atomic>
 #include <chrono>
@@ -383,7 +383,7 @@ class Implementation : private virtual slsDetectorDefs {
     std::vector<std::unique_ptr<DataProcessor>> dataProcessor;
     std::vector<std::unique_ptr<DataStreamer>> dataStreamer;
     std::vector<std::unique_ptr<Fifo>> fifo;
-    std::unique_ptr<ThreadArping> threadArping;
+    std::unique_ptr<Arping> arping;
 
     std::mutex hdf5Lib;
 };

slsReceiverSoftware/src/ThreadObject.h

@@ -21,16 +21,6 @@ class ThreadObject : private virtual slsDetectorDefs {
   protected:
     const int index{0};
 
-  protected:
-    std::atomic<bool> killThread{false};
-
-  private:
-    std::atomic<bool> runningFlag{false};
-    std::thread threadObject;
-    sem_t semaphore;
-    const std::string type;
-    pid_t threadId{0};
-
   public:
     ThreadObject(int threadIndex, std::string threadType);
     virtual ~ThreadObject();
@@ -49,4 +39,11 @@ class ThreadObject : private virtual slsDetectorDefs {
      * Then it exits the thread on its own if killThread is true
      */
     void RunningThread();
+
+    std::atomic<bool> killThread{false};
+    std::atomic<bool> runningFlag{false};
+    std::thread threadObject;
+    sem_t semaphore;
+    const std::string type;
+    pid_t threadId{0};
 };