From 9395fe8058167f86e8ffb2d8fa18661c0e39d9e7 Mon Sep 17 00:00:00 2001
From: aschwinn <al.schwinn@gsi.de>
Date: Mon, 24 Jul 2017 12:13:23 +0200
Subject: [PATCH] [SIL-258] Ping does not work properly

---
 .../interface/communication/CNVConnection.cpp |  30 +++--
 .../communication/SilecsConnection.cpp        | 104 +++++++++---------
 .../communication/SilecsConnection.h          |   2 +-
 .../interface/equipment/SilecsPLC.cpp         |  18 +++
 4 files changed, 87 insertions(+), 67 deletions(-)

diff --git a/silecs-communication-cpp/src/silecs-communication/interface/communication/CNVConnection.cpp b/silecs-communication-cpp/src/silecs-communication/interface/communication/CNVConnection.cpp
index be6ca90..143a805 100644
--- a/silecs-communication-cpp/src/silecs-communication/interface/communication/CNVConnection.cpp
+++ b/silecs-communication-cpp/src/silecs-communication/interface/communication/CNVConnection.cpp
@@ -100,24 +100,20 @@ namespace Silecs
 
 	bool CNVConnection::open(PLC* thePLC)
 	{
-	    bool isOK = false;
+        //Controller is reachable at this stage (ping done from doOpen)
+        bool isOK = true; //all subscription is fine a priori
 
-        if (IeRfcPing((char *) thePLC->getName().c_str(), 0) == 0)
-        {
-            LOG(DEBUG) << "CNVConnection::open(): Controller is reachable (ping), then subscribe input variables";
-
-            isOK = true; //all subscription is fine a priori
+        LOG(DEBUG) << "CNVConnection::open(): Subscribe to all input variables";
 
-            // Create all the subscriptions
-            blockMapType::iterator pBlockIter;
-            for(pBlockIter = thePLC->getBlockMap().begin(); pBlockIter != thePLC->getBlockMap().end(); ++pBlockIter)
-            {
-                //Attention: only CNVInputBlock has doSubscribe/unSubscribe feature (key-map can be used to select appropriate object)
-                if (pBlockIter->first.find(Block::whichAccessType(Input)) != std::string::npos)
-                { if (static_cast<CNVInputBlock*>(pBlockIter->second)->doSubscribe() == false)
-                    {  isOK = false;
-                       break;
-                    }
+        // Create all the subscriptions
+        blockMapType::iterator pBlockIter;
+        for(pBlockIter = thePLC->getBlockMap().begin(); pBlockIter != thePLC->getBlockMap().end(); ++pBlockIter)
+        {
+            //Attention: only CNVInputBlock has doSubscribe/unSubscribe feature (key-map can be used to select appropriate object)
+            if (pBlockIter->first.find(Block::whichAccessType(Input)) != std::string::npos)
+            { if (static_cast<CNVInputBlock*>(pBlockIter->second)->doSubscribe() == false)
+                {  isOK = false;
+                   break;
                 }
             }
         }
@@ -127,7 +123,7 @@ namespace Silecs
 
 	bool CNVConnection::close(PLC* thePLC)
 	{
-        LOG(DEBUG) << "CNVConnection::close(): unSubscribe input variables";
+        LOG(DEBUG) << "CNVConnection::close(): unSubscribe all input variables";
 
 		// Delete all the subscription
 		blockMapType::iterator pBlockIter;
diff --git a/silecs-communication-cpp/src/silecs-communication/interface/communication/SilecsConnection.cpp b/silecs-communication-cpp/src/silecs-communication/interface/communication/SilecsConnection.cpp
index b9ad8cf..77b3abc 100644
--- a/silecs-communication-cpp/src/silecs-communication/interface/communication/SilecsConnection.cpp
+++ b/silecs-communication-cpp/src/silecs-communication/interface/communication/SilecsConnection.cpp
@@ -90,7 +90,9 @@ namespace Silecs
 	bool Connection::doOpen(PLC* thePLC)
 	{
 		//LOG((COMM|DIAG)) << "Start attempt to open connection ..."; .. commented, seem like this creates to many log-entries for KIBANA
-		bool justConnected = false;
+        bool isReachable    = false;
+        bool isOpen         = false;
+        bool justConnected  = false;
 
 		{
 			Lock lock(connMux_);
@@ -111,49 +113,54 @@ namespace Silecs
 
 			if (isTimeToReconnect())
 			{
-				LOG((COMM|DIAG)) << "It's time to reconnect";
-
-				// It's time to open the connection according to the (re)connection timing
-				// Let's try several times with limited delay (some ms).
-				// It allows wake-up frozen PLC (SIEMENS in particular) after long stop period.
-				bool isOpen  = false;
-				unsigned int nbConn = 2;    //for fast low-level iteration
-				for(unsigned int i = 0; i<nbConn; i++)
-				{
-					LOG((COMM|DIAG)) << "Attempt to open PLC connection ....";
-					isOpen = open(thePLC);
-					if(isOpen)
-					{
-						LOG((COMM|DIAG)) << "Connection opened successfully";
-						break;
-					}
-					usleep(100000); // wait 100ms
-				}
-				if(!isOpen)
-				{
-					logError(thePLC);
-					return isConnected_;
-				}
+                if (isReachable = (IeRfcPing((char *) thePLC->getName().c_str(), NULL) == 0))
+                {
+                    LOG((COMM|DIAG)) << "It's time to reconnect";
+
+                    // It's time to open the connection according to the (re)connection timing
+                    // Let's try several times with limited delay (some ms).
+                    // It allows wake-up frozen PLC (SIEMENS in particular) after long stop period.
+                    bool isOpen  = false;
+                    unsigned int nbConn = 2;    //for fast low-level iteration
+                    for(unsigned int i = 0; i<nbConn; i++)
+                    {
+                        LOG((COMM|DIAG)) << "Attempt to open PLC connection ....";
+                        isOpen = open(thePLC);
+                        if(isOpen)
+                        {
+                            LOG((COMM|DIAG)) << "Connection opened successfully";
+                            break;
+                        }
+                        usleep(100000); // wait 100ms
+                    }
+                }//pingable?
+
+                if(!isOpen)
+                {
+                    logError(thePLC,isReachable);
+                    return isConnected_;
+                }
+
+                if (thePLC->isSharedConnection())
+                {
+                    LOG((COMM|DIAG)) << "Shared connection with " << thePLC->getName() << " is established.";
+                }
+                else
+                {
+                    LOG((COMM|DIAG)) << "Connection with " << thePLC->getName() <<
+                                      ":" << thePLC->theCluster_->getClassName() <<
+                                      "/v" << thePLC->theCluster_->getClassVersion() <<
+                                      " is established.";
+                }
+
+                isAlive_ = true;
+                isConnected_ = true;
+                justConnected = true; //retentive registers synchronization is required!
+
+                //Connection status has changed: update the diagnostic variables
+                LOG((COMM|DIAG)) << "Updating PLC status";
+                updateStatus(thePLC);
 
-				if (thePLC->isSharedConnection())
-				{
-					LOG((COMM|DIAG)) << "Shared connection with " << thePLC->getName() << " is established.";
-				}
-				else
-				{
-					LOG((COMM|DIAG)) << "Connection with " << thePLC->getName() <<
-									  ":" << thePLC->theCluster_->getClassName() <<
-									  "/v" << thePLC->theCluster_->getClassVersion() <<
-									  " is established.";
-				}
-
-				isAlive_ = true;
-				isConnected_ = true;
-				justConnected = true; //retentive registers synchronization is required!
-
-				//Connection status has changed: update the diagnostic variables
-				LOG((COMM|DIAG)) << "Updating PLC status";
-				updateStatus(thePLC);
 			}
 		}//release lock
 
@@ -323,12 +330,11 @@ namespace Silecs
 
 
 	//------------------------------------------------------------------------------------------------------------------------------------------------
-	void Connection::logError(PLC* thePLC)
-	{
-	    std::string errorMsg = "Connection with " + thePLC->getName() +
-							   ":" + thePLC->theCluster_->getClassName() +
-								"/v" + thePLC->theCluster_->getClassVersion() +
-								" has failed. ";
+    void Connection::logError(PLC* thePLC, bool isReachable)
+    {
+        std::string errorMsg = isReachable ? "Connection with " + thePLC->getName() + ":" + thePLC->theCluster_->getClassName() +
+                                              "/v" + thePLC->theCluster_->getClassVersion() + " has failed.\n"
+                                           : "Controller " + thePLC->getName() + " does not respond to ping, might be OFF!\n";
 
 		if (delayConn_ == LongTermConnection)
 		{	if (remainConn_ > 0)
diff --git a/silecs-communication-cpp/src/silecs-communication/interface/communication/SilecsConnection.h b/silecs-communication-cpp/src/silecs-communication/interface/communication/SilecsConnection.h
index d168f7c..7d350c7 100644
--- a/silecs-communication-cpp/src/silecs-communication/interface/communication/SilecsConnection.h
+++ b/silecs-communication-cpp/src/silecs-communication/interface/communication/SilecsConnection.h
@@ -133,7 +133,7 @@ namespace Silecs
          */
 		void updateStatus(PLC* thePLC);
 
-		void logError(PLC* thePLC);
+        void logError(PLC* thePLC, bool isReachable);
 
         /*!
          * \fn isTimeToReconnect
diff --git a/silecs-communication-cpp/src/silecs-communication/interface/equipment/SilecsPLC.cpp b/silecs-communication-cpp/src/silecs-communication/interface/equipment/SilecsPLC.cpp
index 1cc2c4d..edc5896 100644
--- a/silecs-communication-cpp/src/silecs-communication/interface/equipment/SilecsPLC.cpp
+++ b/silecs-communication-cpp/src/silecs-communication/interface/equipment/SilecsPLC.cpp
@@ -143,6 +143,24 @@ namespace Silecs
 					throw SilecsException(__FILE__, __LINE__, CONFIG_CLIENT_PLC_NOT_CONSISTENT, message.str());
 				}
 			}
+
+            //Warn ACET service that a new connection is established (except for SilecsHeader)
+            if (theHeader_ != NULL) {
+                TRACE("cluster") << "ClassName=" << theCluster_->getClassName() << "|" <<
+                                    "ClassVersion=" << theCluster_->getClassVersion() << "|" <<
+                                    "Owner=" << localOwner_ << "|" <<
+                                    "GenerationRelease=" << localRelease_ << "|" <<
+                                    "GenerationDate=" << localDate_ << "|" <<
+                                    "PlcChecksum=" << localChecksum_ << "|" <<
+                                    "PlcDomain=" << domain_ << "|" <<
+                                    "PlcHostname=" << name_ << "|" <<
+                                    "PlcBrand=" << brand_ << "|" <<
+                                    "PlcSystem=" << system_ << "|" <<
+                                    "PlcModel=" << model_ << "|" <<
+                                    "ProtocolAddress=" << baseAddr_ << "|" <<
+                                    "ProtocolMode=" << protocolMode_ << "|" <<
+                                    "InstanceNumber=" << deviceCol_.size();
+            }
 		};
 
 
-- 
GitLab