Merged #162

ChangeLog.md

@@ -1,5 +1,6 @@
 ## Changes between ifm3d 0.15.0 and 0.15.1
 * Minor updates to allow for cross-compiling ifm3d for the O3D3XX
+*  PCIC timeout issue fixed
 
 ## Changes between ifm3d 0.14.1 and 0.15.0
 * Added Interface for getting json_model from O3D3xx devices.

modules/pcicclient/include/ifm3d/pcicclient/pcicclient.h

@@ -32,7 +32,7 @@ namespace ifm3d
    * ifm3d::Camera::Ptr, it provides unbuffered communication with
    * the PCIC interface.
    */
-   
+
   class PCICClient
   {
   public:
@@ -101,9 +101,36 @@ namespace ifm3d
      *
      * @return Copy of received plain response data as string
      * (without any header information, like ticket, length, etc.)
+     *
+     * NOTE: This Call can block and hang indefinitely depending upon PCIC response.
      */
     std::string Call(const std::string& request);
 
+    /**
+     * Similar to the Call function above.
+     *
+     * Sends a PCIC command to the camera and returns the response
+     * as soon as it has been received. In the meanwhile, this call
+     * is blocked.
+     *
+     * @param[in] request String containing the plain command
+     * (without any header infomration, like ticket, length, etc.)
+     *
+     * @param[in] response String containing the response from the camera
+     *  providing the plain response data as string
+     * (without any header information, like ticket, length, etc.)
+     *
+     * @param[in] timeout in milliseconds, in case, the PCIC fails to come
+     * through.
+     *
+     * NOTE: This Call can fail with no response if supplied with an
+     * unsuitable "timeout_millis" value. Providing timeout_millis value as 0
+     * results in behaviour similar to the above Call method.
+     *
+     * @return true if Call succeeded, false if failed.
+     */
+    bool Call(const std::string& request, std::string &response, long timeout_millis);
+
     /**
      * Sets the specified callback for receiving asynchronous error messages
      * until it is replaced by a new callback or canceled via

modules/pcicclient/src/libifm3d_pcicclient/pcicclient.cpp

@@ -50,6 +50,13 @@ ifm3d::PCICClient::Call(const std::string& request)
 	return this->pImpl->Call(request);
 }
 
+bool
+ifm3d::PCICClient::Call(const std::string& request,
+			 std::string& response, long timeout_millis)
+{
+	return this->pImpl->Call(request, response, timeout_millis);
+}
+
 long
 ifm3d::PCICClient
 ::SetErrorCallback(std::function<void(const std::string& error)> callback)

modules/pcicclient/src/libifm3d_pcicclient/pcicclient_impl.hpp

@@ -49,7 +49,7 @@ namespace ifm3d
 	const int PRE_CONTENT_BUFFER_LENGTH = 20;
 	const int POST_CONTENT_BUFFER_LENGTH = 2;
 
- 
+
   //============================================================
   // Impl interface
   //============================================================
@@ -102,9 +102,36 @@ namespace ifm3d
 	*
 	* @return Copy of received plain response data as string
 	* (without any header information, like ticket, length, etc.)
+	*
+	* NOTE: This Call can block and hang indefinitely depending upon PCIC response.
 	*/
 	std::string Call(const std::string& request);
 
+	/**
+	* Similar to the Call function above.
+	*
+	* Sends a PCIC command to the camera and returns the response
+	* as soon as it has been received. In the meanwhile, this call
+	* is blocked.
+	*
+	* @param[in] request String containing the plain command
+	* (without any header infomration, like ticket, length, etc.)
+	*
+	* @param[in] response String containing the response from the camera
+	*  providing the plain response data as string
+	* (without any header information, like ticket, length, etc.)
+	*
+	* @param[in] timeout in milliseconds, in case, the PCIC fails to come
+	* through.
+	*
+	* NOTE: This Call can fail with no response if supplied with an
+	* unsuitable "timeout_millis" value. Providing timeout_millis value as 0
+	* results in behaviour similar to the above Call method.
+	*
+	* @return true if Call succeeded, false if failed.
+	*/
+	bool Call(const std::string& request, std::string &response, long timeout_millis);
+
 	/**
 	* Sets the specified callback for receiving asynchronous error messages
 	* until it is replaced by a new callback or canceled via
@@ -285,7 +312,7 @@ namespace ifm3d
      * Using callback ids for cancelling callbacks instead of tickets eliminates
      * the risk of cancelling a newer callback with the same ticket.
      */
-    long current_callback_id_;
+    unsigned long current_callback_id_;
 
     /**
      * Maps PCIC tickets to callback ids. When receiving an incoming message,
@@ -501,26 +528,75 @@ ifm3d::PCICClient::Impl::Call(const std::string& request,
 std::string
 ifm3d::PCICClient::Impl::Call(const std::string& request)
 {
-	std::atomic_bool has_result(false);
-	std::string result;
+  std::string response;
+  Call(request, response, 0);
+  return response;
+}
 
-	Call(request, [&](const std::string& content)
-	{
-		// Copy content, notify and leave callback
-		result = content;
-		std::unique_lock<std::mutex> lock(this->in_mutex_);
-		has_result.store(true);
-		this->in_cv_.notify_all();
-	});
-
-	std::unique_lock<std::mutex> lock(this->in_mutex_);
-	while (!has_result.load())
-	{
-		this->in_cv_.wait(lock);
-	}
-	lock.unlock();
+bool/* @R Consider boost::asio::error return type here */
+ifm3d::PCICClient::Impl::Call(const std::string& request, std::string& response, long timeout_millis)
+{
+  std::atomic_bool has_result(false);
+  long call_output = -1;
+
+  // Handle the PCIC Call
 
-	return result;
+  std::unique_ptr<std::thread> call_thread_ = std::make_unique<std::thread>([&]{
+  call_output = Call(request, [&](const std::string& content)
+  {
+    // Copy content, notify and leave callback
+    response = content;
+    std::unique_lock<std::mutex> lock(this->in_mutex_);
+    has_result.store(true);
+    this->in_cv_.notify_all();
+
+  });
+
+  });
+
+  if(call_thread_ && call_thread_->joinable()) call_thread_->join();
+
+  // Check the return value of our PCIC Call
+  if(call_output > 0)
+  {
+    std::unique_lock<std::mutex> lock(this->in_mutex_);
+    try
+    {
+      if (timeout_millis <= 0)
+      {
+        this->in_cv_.wait(lock, [&]{return has_result.load();});
+      }
+
+      else
+      {
+        if (this->in_cv_.wait_for(
+              lock, std::chrono::milliseconds(timeout_millis)) ==
+            std::cv_status::timeout)
+        {
+          this->in_cv_.notify_all();
+          if(this->thread_ && this->thread_->joinable())
+          {
+          	this->Stop();
+          	this->thread_->join();
+          }
+          return has_result.load();
+        }
+
+        else
+        {
+          this->in_cv_.wait(lock, [&]{return has_result.load();});
+        }
+      }
+    }
+
+    catch (const std::system_error& ex)
+    {
+      LOG(WARNING) << "PCICClient::Call: " << ex.what();
+      return has_result.load();
+    }
+  }
+
+  return has_result.load();
 }
 
 long
@@ -786,13 +862,7 @@ ifm3d::PCICClient::Impl::NextCommandTicket()
 long
 ifm3d::PCICClient::Impl::NextCallbackId()
 {
-  // In case of long overflow, reset to 1
-  if(++this->current_callback_id_ <= 0)
-    {
-      this->current_callback_id_ = 1;
-    }
-
-  return this->current_callback_id_;
+  return (this->current_callback_id_ == 0)? (this->current_callback_id_ = 1) : (++this->current_callback_id_) ;
 }
 
 

modules/pcicclient/test/ifm3d-pcicclient-tests.cpp

@@ -16,7 +16,7 @@ class PCICClientTest : public ::testing::Test
 
   virtual void TearDown()
   {
-
+    
   }
 
   ifm3d::Camera::Ptr cam_;
@@ -67,3 +67,43 @@ TEST_F(PCICClientTest, InvalidCommandLength)
 
   pc->Stop();
 }
+
+TEST_F(PCICClientTest, PCICTimeout)
+{
+  std::string result;
+  //
+  // PCICClientTest is not supported for O3X
+  // so this test does not apply
+  //
+  if (cam_->IsO3X())
+    {
+      EXPECT_THROW(std::make_shared<ifm3d::PCICClient>(cam_), ifm3d::error_t);
+      return;
+    }
+
+  ifm3d::PCICClient::Ptr pc = std::make_shared<ifm3d::PCICClient>(cam_);    
+
+  std::unique_ptr<std::thread> reboot_thread_ = std::make_unique<std::thread>([&]{
+
+  EXPECT_NO_THROW(this->cam_->Reboot(ifm3d::Camera::boot_mode::PRODUCTIVE));
+
+  std::this_thread::sleep_for(std::chrono::seconds(1));
+
+  });  
+
+  if(reboot_thread_ && reboot_thread_->joinable()) reboot_thread_->join();  
+
+  for(int i = 0; i < 20; ++i)
+    {    
+      result.clear();
+
+      if(!pc->Call("V?",result,5000))
+       {     
+         pc = std::make_shared<ifm3d::PCICClient>(cam_);
+       }    
+    }
+
+  EXPECT_GT(result.size(), 0);
+  pc->Stop();
+
+}