Merge pull request #11 from nabto/qa

Qa

Author: fusionfoxy

README.md

@@ -60,3 +60,7 @@ Run the RTSP device:
    -k key.pem --secret <secret> \
    -r rtsp://127.0.0.1:8554/video`
 ```
+
+## Limitations
+
+The libdatachannel WebRTC library used by the examples does not currently support offers generated with `restartIce()` due to this [issue](https://github.com/paullouisageneau/libdatachannel/issues/545). This means if one of the peers experiences a network failure or switches to a different network, any open WebRTC connections cannot be renegotiated. Instead the connection has to be recreated from scratch.

examples/libdatachannel/src/common/webrtc_connection/webrtc_connection.cpp

@@ -109,8 +109,13 @@ void WebrtcConnection::handleMessage(const nlohmann::json& msg) {
                   "discarding the received offer";
         return;
       }
-
-      pc_->setRemoteDescription(remDesc);
+      try {
+        pc_->setRemoteDescription(remDesc);
+      } catch (std::exception& ex) {
+        NPLOGE << "Failed to set remote description: " << remDesc.generateSdp()
+               << " Failed with exception: " << ex.what();
+        pc_->close();
+      }
     } else if (type == "CANDIDATE") {
       try {
         rtc::Candidate cand(
@@ -195,7 +200,7 @@ void WebrtcConnection::handleStateChange(
       // \o/ we should use the connection
       break;
     case rtc::PeerConnection::State::Failed:
-      // connection fail and we should fix it
+      // connection failed
     case rtc::PeerConnection::State::Closed:
       // connection closed we should clean up
       NPLOGI << "Webrtc Connection " << state;

sdk/src/signaling_device/include/nabto/webrtc/device.hpp

@@ -293,13 +293,18 @@ class SignalingTimer {
   SignalingTimer& operator=(SignalingTimer&&) = delete;
 
   /**
-   * Set a timeout in ms at which the callback sould be invoked.
+   * Set a timeout in ms at which the callback should be invoked.
    *
    * @param timeoutMs The timeout in milliseconds.
    * @param callback The callback to be invoked once the timeout has passed.
    */
   virtual void setTimeout(uint32_t timeoutMs,
                           std::function<void()> callback) = 0;
+
+  /**
+   * Cancel a started timer.
+   */
+  virtual void cancel() = 0;
 };
 
 /**

sdk/src/signaling_device/src/signaling_device_impl.cpp

@@ -171,14 +171,29 @@ void SignalingDeviceImpl::close() {
   {
     const std::lock_guard<std::mutex> lock(mutex_);
     channels_.clear();
-    chanHandlers_.clear();
-    stateHandlers_.clear();
-    reconnHandlers_.clear();
 
     if (ws_) {
       ws_->close();
     }
   }
+  if (!ws_) {
+    deinit();
+  }
+}
+
+void SignalingDeviceImpl::deinit() {
+  SignalingTimerPtr timer = nullptr;
+  {
+    const std::lock_guard<std::mutex> lock(mutex_);
+    timer = timer_;
+  }
+  if (timer) {
+    timer->cancel();
+  }
+  const std::lock_guard<std::mutex> lock(mutex_);
+  chanHandlers_.clear();
+  stateHandlers_.clear();
+  reconnHandlers_.clear();
 }
 
 void SignalingDeviceImpl::connectWs() {
@@ -212,6 +227,8 @@ void SignalingDeviceImpl::connectWs() {
     if (self->state_ != SignalingDeviceState::CLOSED &&
         self->state_ != SignalingDeviceState::FAILED) {
       self->waitReconnect();
+    } else {
+      self->deinit();
     }
   });
 
@@ -220,6 +237,8 @@ void SignalingDeviceImpl::connectWs() {
     if (self->state_ != SignalingDeviceState::CLOSED &&
         self->state_ != SignalingDeviceState::FAILED) {
       self->waitReconnect();
+    } else {
+      self->deinit();
     }
   });
 

sdk/src/signaling_device/src/signaling_device_impl.hpp

@@ -110,6 +110,8 @@ class SignalingDeviceImpl
   SignalingDeviceState state_ = SignalingDeviceState::NEW;
   std::mutex mutex_;
 
+  void deinit();
+
   // WS STUFF
   WebsocketConnectionPtr ws_;
   size_t reconnectCounter_ = 0;

sdk/src/signaling_device/src/websocket_connection.hpp

@@ -30,6 +30,18 @@ class WebsocketConnection
                       SignalingTimerFactoryPtr timerFactory)
       : ws_(std::move(websocket)), timerFactory_(std::move(timerFactory)) {}
 
+  ~WebsocketConnection() {
+    if (timer_) {
+      timer_->cancel();
+    }
+  }
+
+  WebsocketConnection() = default;
+  WebsocketConnection(const WebsocketConnection&) = delete;
+  WebsocketConnection& operator=(const WebsocketConnection&) = delete;
+  WebsocketConnection(WebsocketConnection&&) = delete;
+  WebsocketConnection& operator=(WebsocketConnection&&) = delete;
+
   bool send(const std::string& data);
   void close();
   void onOpen(std::function<void()> callback);
@@ -46,7 +58,7 @@ class WebsocketConnection
   SignalingWebsocketPtr ws_;
   SignalingTimerFactoryPtr timerFactory_;
   size_t pongCounter_ = 0;
-  SignalingTimerPtr timer_;
+  SignalingTimerPtr timer_ = nullptr;
 
   void handlePong();
   static SignalingMessageType parseWsMsgType(const std::string& str);

sdk/src/signaling_util/curl_http_client/include/nabto/webrtc/util/curl_async.hpp

@@ -30,6 +30,12 @@ class CurlHttpClient : public nabto::signaling::SignalingHttpClient,
   static nabto::signaling::SignalingHttpClientPtr create();
 
   CurlHttpClient();
+  ~CurlHttpClient() override;
+  CurlHttpClient(const CurlHttpClient&) = delete;
+  CurlHttpClient& operator=(const CurlHttpClient&) = delete;
+  CurlHttpClient(CurlHttpClient&&) = delete;
+  CurlHttpClient& operator=(CurlHttpClient&&) = delete;
+
   bool sendRequest(const nabto::signaling::SignalingHttpRequest& request,
                    nabto::signaling::HttpResponseCallback cb) override;
 

sdk/src/signaling_util/curl_http_client/src/curl_async.cpp

@@ -23,6 +23,13 @@ nabto::signaling::SignalingHttpClientPtr CurlHttpClient::create() {
 
 CurlHttpClient::CurlHttpClient() : curl_(CurlAsync::create()) {}
 
+CurlHttpClient::~CurlHttpClient() {
+  if (curlReqHeaders_ != nullptr) {
+    curl_slist_free_all(curlReqHeaders_);
+    curlReqHeaders_ = nullptr;
+  }
+}
+
 bool CurlHttpClient::sendRequest(
     const nabto::signaling::SignalingHttpRequest& request,
     nabto::signaling::HttpResponseCallback cb) {
@@ -157,10 +164,8 @@ CurlAsyncPtr CurlAsync::create() {
 }
 
 CurlAsync::~CurlAsync() {
-  try {
-    thread_ = std::thread();
-  } catch (std::exception& ex) {
-    NPLOGE << "~CurlAsync exception " << ex.what();
+  if (!stopped_) {
+    stop();
   }
   curl_easy_cleanup(curl_);
   curl_global_cleanup();

sdk/src/signaling_util/message_transport/src/message_transport.cpp

@@ -3,8 +3,6 @@
 #include <nabto/webrtc/device.hpp>
 #include <nabto/webrtc/util/message_transport.hpp>
 
-#include <functional>
-#include <string>
 #include <utility>
 
 namespace nabto {

sdk/src/signaling_util/std_timer/include/nabto/webrtc/util/std_timer.hpp

@@ -15,11 +15,7 @@ namespace util {
 class StdTimer : public nabto::signaling::SignalingTimer,
                  public std::enable_shared_from_this<StdTimer> {
  public:
-  ~StdTimer() {
-    if (timer_.joinable()) {
-      timer_.join();
-    }
-  }
+  ~StdTimer() {}
 
   void setTimeout(uint32_t timeoutMs, std::function<void()> cb) override {
     auto self = shared_from_this();
@@ -30,6 +26,13 @@ class StdTimer : public nabto::signaling::SignalingTimer,
     });
   }
 
+  void cancel() override {
+    if (timer_.joinable()) {
+      timer_.join();
+      timer_ = std::thread();
+    }
+  }
+
  private:
   std::thread timer_;
 };