Circumenting bug in indiserver when removing/re-adding device by crea…

…ting a new INDI client object.

source/indi-device-watchdog/indi_auto_connector.cpp

@@ -36,55 +36,86 @@
 
 #include "indi_auto_connector.h"
 
-IndiAutoConnectorT::IndiAutoConnectorT(const std::string & hostname, int port, const std::vector<DeviceDataT> & devicesToMonitor) {
+IndiAutoConnectorT::IndiAutoConnectorT(const std::string & hostname, int port, const std::vector<DeviceDataT> & devicesToMonitor) : hostname_(hostname), port_(port) {
   using namespace std::chrono_literals;
 
+  resetIndiClient();
+
   // Process config entries to deviceConnections_
   for (auto it = devicesToMonitor.begin(); it != devicesToMonitor.end(); ++it) {
     deviceConnections_.insert( std::pair<std::string, DeviceDataT>(it->getIndiDeviceName(), *it) );
   }
 
-  client_.setServer(hostname.c_str(), port);
+}
+
+IndiAutoConnectorT::~IndiAutoConnectorT() {
+
+  serverConnectionFailedListenerConnection_.disconnect();
+  //serverConnectionStateChangedConnection_.disconnect();
+  newDeviceListenerConnection_.disconnect();
+  removeDeviceListenerConnection_.disconnect();
+  newPropertyListenerConnection_.disconnect();
+  removePropertyListenerConnection_.disconnect();
+  updatePropertyListenerConnection_.disconnect();
+
+  client_->disconnect();
+}
+
+
+void IndiAutoConnectorT::resetIndiClient() {
+
+  serverConnectionFailedListenerConnection_.disconnect();
+  //serverConnectionStateChangedConnection_.disconnect();
+  newDeviceListenerConnection_.disconnect();
+  removeDeviceListenerConnection_.disconnect();
+  newPropertyListenerConnection_.disconnect();
+  removePropertyListenerConnection_.disconnect();
+  updatePropertyListenerConnection_.disconnect();
+
+  if (client_ != nullptr) {
+    client_->disconnect();
+  }
+
+  std::cerr << "Resetting INDI client..." << std::endl;
+
+  connected_ = false;
+
+  client_ = std::make_shared<IndiClientT>(); // Create a new client
 
-  serverConnectionFailedListenerConnection_ = client_.registerServerConnectionFailedListener([&]() {
+  client_->setServer(hostname_.c_str(), port_);
+
+  serverConnectionFailedListenerConnection_ = client_->registerServerConnectionFailedListener([&]() {
     std::cerr << "Connection to INDI server failed." << std::endl;
     connected_ = false;
   });
 
-  newDeviceListenerConnection_ = client_.registerNewDeviceListener([&](INDI::BaseDevice device) {
+  // serverConnectionStateChangedConnection_ = client_->registerServerConnectionStateChangedListener([&](IndiServerConnectionStateT::TypeE indiServerConnectionState) {
+  //   std::cout << "Connection to INDI changed to " << IndiServerConnectionStateT::asStr(indiServerConnectionState) << std::endl;
+
+  //   connected_ = (indiServerConnectionState == IndiServerConnectionStateT::CONNECTED);
+  // });
+
+  newDeviceListenerConnection_ = client_->registerNewDeviceListener([&](INDI::BaseDevice device) {
     addIndiDevice(device);
   });
 
-  removeDeviceListenerConnection_ = client_.registerRemoveDeviceListener([&](INDI::BaseDevice device) {
+  removeDeviceListenerConnection_ = client_->registerRemoveDeviceListener([&](INDI::BaseDevice device) {
     removeIndiDevice(device);
   });
 
-  newPropertyListenerConnection_ = client_.registerNewPropertyListener([&](INDI::Property property) {
+  newPropertyListenerConnection_ = client_->registerNewPropertyListener([&](INDI::Property property) {
     propertyUpdated(property);
   });
 
-  removePropertyListenerConnection_ = client_.registerRemovePropertyListener([&](INDI::Property property) {
+  removePropertyListenerConnection_ = client_->registerRemovePropertyListener([&](INDI::Property property) {
     propertyRemoved(property);
   });
 
-  updatePropertyListenerConnection_ = client_.registerUpdatePropertyListener([&](INDI::Property property) {
+  updatePropertyListenerConnection_ = client_->registerUpdatePropertyListener([&](INDI::Property property) {
     propertyUpdated(property);
   });
 }
 
-IndiAutoConnectorT::~IndiAutoConnectorT() {
-
-  serverConnectionFailedListenerConnection_.disconnect();
-  newDeviceListenerConnection_.disconnect();
-  removeDeviceListenerConnection_.disconnect();
-  newPropertyListenerConnection_.disconnect();
-  removePropertyListenerConnection_.disconnect();
-  updatePropertyListenerConnection_.disconnect();
-
-  client_.disconnect();
-}
-
-
 INDI::BaseDevice IndiAutoConnectorT::getBaseDeviceFromProperty(INDI::Property property) {
 #if INDI_MAJOR_VERSION < 2
   return *property.getBaseDevice();
@@ -173,12 +204,14 @@ void IndiAutoConnectorT::propertyUpdated(INDI::Property /*property*/) {
 }
 
 
-void IndiAutoConnectorT::requestIndiDriverRestart(DeviceDataT & deviceData) {
+bool IndiAutoConnectorT::requestIndiDriverRestart(DeviceDataT & deviceData) {
   std::string driverName = deviceData.getIndiDeviceDriverName();
 
-  indiDriverRestartManager_.requestRestart(driverName);
+  bool restarted = indiDriverRestartManager_.requestRestart(driverName);
 
   deviceData.setIndiBaseDevice(INDI::BaseDevice());
+
+  return restarted;
 }
 
 
@@ -198,7 +231,7 @@ bool IndiAutoConnectorT::isDeviceValid(INDI::BaseDevice indiBaseDevice) {
  */
 bool IndiAutoConnectorT::requestConnectionStateChange(INDI::BaseDevice indiBaseDevice, bool connect) {
 
-  std::cerr << "Sending INDI device connect request for device ' '" << indiBaseDevice.getDeviceName() << "'..." << std::endl;
+  std::cerr << "Sending INDI device " << (connect ? "connect" : " disconnect") << " request for device ' '" << indiBaseDevice.getDeviceName() << "'..." << std::endl;
 
   if (! isDeviceValid(indiBaseDevice)) {
     return false;
@@ -218,7 +251,7 @@ bool IndiAutoConnectorT::requestConnectionStateChange(INDI::BaseDevice indiBaseD
     connectionSwitchVec->sp[0].s = (connect ? ISS_ON : ISS_OFF);
     connectionSwitchVec->sp[1].s = (connect ? ISS_OFF : ISS_ON);
 
-    client_.sendNewSwitch(connectionSwitchVec);
+    client_->sendNewSwitch(connectionSwitchVec);
 
 #else    
   INDI::PropertySwitch connectionSwitch = indiBaseDevice.getSwitch("CONNECTION");
@@ -231,7 +264,7 @@ bool IndiAutoConnectorT::requestConnectionStateChange(INDI::BaseDevice indiBaseD
   connectionSwitch[0].setState(connect ? ISS_ON : ISS_OFF);
   connectionSwitch[1].setState(connect ? ISS_OFF : ISS_ON);
 
-  client_.sendNewSwitch(connectionSwitch);
+  client_->sendNewSwitch(connectionSwitch);
 #endif
 
   return true;
@@ -255,7 +288,7 @@ bool IndiAutoConnectorT::isIndiDeviceConnected(INDI::BaseDevice indiBaseDevice)
 }
 
 
-void IndiAutoConnectorT::handleDeviceConnection(DeviceDataT & deviceData) {
+bool IndiAutoConnectorT::handleDeviceConnection(DeviceDataT & deviceData) {
   std::string indiDeviceName = deviceData.getIndiDeviceName();
 
   bool indiDeviceConnected = isIndiDeviceConnected(deviceData.getIndiBaseDevice());
@@ -269,7 +302,7 @@ void IndiAutoConnectorT::handleDeviceConnection(DeviceDataT & deviceData) {
     if (! indiDeviceExists) {
       // Linux device is there but corresponding INDI base
       // device does not exist -> Restart INDI driver
-      requestIndiDriverRestart(deviceData);
+      return requestIndiDriverRestart(deviceData);
     }
     else {
       // Linux device is there and corresponding INDI base
@@ -281,7 +314,7 @@ void IndiAutoConnectorT::handleDeviceConnection(DeviceDataT & deviceData) {
 
 	if (! successful) {
 	  // If connection fails, restart INDI driver
-	  requestIndiDriverRestart(deviceData);
+	  return requestIndiDriverRestart(deviceData);
 	}
       }
     }
@@ -296,11 +329,12 @@ void IndiAutoConnectorT::handleDeviceConnection(DeviceDataT & deviceData) {
 
       if (! successful) {
 	// If disconnect fails, restart INDI driver
-	requestIndiDriverRestart(deviceData);
+	return requestIndiDriverRestart(deviceData);
       }
     }
   }
-
+
+  return false;
 }
 
 
@@ -313,10 +347,10 @@ void IndiAutoConnectorT::run() {
     std::cerr << "Trying to connect to INDI server...";
 
     // Try to (re-) connect to the INDI server
-    client_.connect();
+    client_->connect();
 
     auto isClientConnected = [&]() -> bool {
-      return client_.isConnected();
+      return client_->isConnected();
     };
 
     try {
@@ -334,9 +368,31 @@ void IndiAutoConnectorT::run() {
       std::this_thread::sleep_for(std::chrono::milliseconds(5000ms));
 
       std::lock_guard<std::mutex> guard(deviceConnectionsMutex_);
-
+      
       for (auto it = deviceConnections_.begin(); it != deviceConnections_.end(); ++it) {
-	handleDeviceConnection(it->second);
+	bool restarted = handleDeviceConnection(it->second);
+
+	if (restarted) {
+
+	  resetIndiClient();
+	  // std::this_thread::sleep_for(std::chrono::milliseconds(3000ms));
+
+	  // client_->disconnect();
+
+	  // auto isClientDisconnected = [&]() -> bool {
+	  //   return !client_->isConnected();
+	  // };
+
+	  // try {
+	  //   wait_for(isClientDisconnected, 5000ms);
+	  //   connected_ = false;
+	  // } catch (std::runtime_error & exc) {
+	  //   std::cerr << "Timeout! Assuming disconnected." << std::endl;
+	  //   connected_ = false;
+	  // }
+
+	  break;
+	}
       }
 
       std::cerr << std::endl << std::endl;

source/indi-device-watchdog/indi_auto_connector.h

@@ -30,6 +30,7 @@
 
 #include <boost/signals2.hpp>
 #include <map>
+#include <memory>
 #include <mutex>
 
 #include "indi_client.h"
@@ -41,14 +42,17 @@
  */
 class IndiAutoConnectorT {
  private:
-  IndiClientT client_;
+  std::string hostname_;
+  int port_;
+  std::shared_ptr<IndiClientT> client_;
   bool connected_;
   boost::signals2::connection serverConnectionFailedListenerConnection_;
   boost::signals2::connection newDeviceListenerConnection_;
   boost::signals2::connection removeDeviceListenerConnection_;
   boost::signals2::connection newPropertyListenerConnection_;
   boost::signals2::connection removePropertyListenerConnection_;
   boost::signals2::connection updatePropertyListenerConnection_;
+  //  boost::signals2::connection serverConnectionStateChangedConnection_;
 
   typedef std::map<std::string /*device name*/, DeviceDataT> DeviceConnStateMapT;
   DeviceConnStateMapT deviceConnections_; 
@@ -59,19 +63,20 @@ class IndiAutoConnectorT {
 
   static bool isDeviceValid(INDI::BaseDevice indiBaseDevice);
   static INDI::BaseDevice getBaseDeviceFromProperty(INDI::Property property);
+  void resetIndiClient();
 
   void addIndiDevice(INDI::BaseDevice device);
   void removeIndiDevice(INDI::BaseDevice device);
   void propertyUpdated(INDI::Property property);
   void propertyRemoved(INDI::Property property);
 
 
-  void requestIndiDriverRestart(DeviceDataT & deviceData);
+  bool requestIndiDriverRestart(DeviceDataT & deviceData);
   bool requestConnectionStateChange(INDI::BaseDevice indiBaseDevice, bool connect);
   bool sendIndiDeviceDisconnectRequest(INDI::BaseDevice indiBaseDevice);
   bool fileExists(const std::string & pathToFile) const;
   static bool isIndiDeviceConnected(INDI::BaseDevice indiBaseDevice);
-  void handleDeviceConnection(DeviceDataT & deviceData);
+  bool handleDeviceConnection(DeviceDataT & deviceData);
 
 
  public:

source/indi-device-watchdog/indi_client.cpp

@@ -34,7 +34,9 @@
 
 IndiClientT::IndiClientT() = default;
 
-IndiClientT::~IndiClientT() = default;
+IndiClientT::~IndiClientT() {
+  disconnect();
+}
 
 
 #if INDI_MAJOR_VERSION < 2
@@ -171,10 +173,11 @@ void IndiClientT::connect() {
 }
 
 void IndiClientT::disconnect() {
-    notifyServerConnectionStateChanged(IndiServerConnectionStateT::DISCONNECTING);
 
     if (this->isServerConnected()) {
-        this->disconnectServer();
+      notifyServerConnectionStateChanged(IndiServerConnectionStateT::DISCONNECTING);
+
+      this->disconnectServer();
     }
 
 }

source/indi-device-watchdog/indi_driver_restart_manager.cpp

@@ -57,9 +57,6 @@ void IndiDriverRestartManagerT::restart(const std::string & indiDriverName) {
     indiServerPipe << "stop " << indiDriverPath.string() << std::endl;
     indiServerPipe << "start " << indiDriverPath.string() << std::endl;
 
-    // TODO: Remove... / Check?
-    std::cerr << "RDSTATE: " << indiServerPipe.rdstate() << std::endl;
-
     indiServerPipe.close();
   }
   else {
@@ -68,15 +65,17 @@ void IndiDriverRestartManagerT::restart(const std::string & indiDriverName) {
 }
 
 
-void IndiDriverRestartManagerT::requestRestart(const std::string & indiDriverName) {
+bool IndiDriverRestartManagerT::requestRestart(const std::string & indiDriverName) {
   auto it = driverRestartMap_.find(indiDriverName);
+  bool restarted = false;
 
   if (it != driverRestartMap_.end()) {
     // Already known..
     bool isRestartConditionReached = (it->second >= restartTriggerLimit_ - 1);
 
     if (isRestartConditionReached) {
       restart(indiDriverName);
+      restarted = true;
 
       // Reset the counter
       it->second = 0;
@@ -89,10 +88,12 @@ void IndiDriverRestartManagerT::requestRestart(const std::string & indiDriverNam
     // New entry
     // First request -> directly restart
     restart(indiDriverName);
+    restarted = true;
 
     driverRestartMap_.insert(std::pair<std::string, int> (indiDriverName, 0));
   }
-
+
+  return restarted;
 }
 
 

source/indi-device-watchdog/indi_driver_restart_manager.h

@@ -41,7 +41,7 @@ class IndiDriverRestartManagerT {
   IndiDriverRestartManagerT();
   IndiDriverRestartManagerT(int restartTriggerLimit);
 
-  void requestRestart(const std::string & indiDriverName);
+  bool requestRestart(const std::string & indiDriverName);
   void requestImmediateRestart(const std::string & indiDriverName);
   void reset();
 };