pr comments, actually use new logic when creating robotclient

Author: stuqdog

src/viam/sdk/robot/client.cpp

@@ -256,7 +256,7 @@ std::shared_ptr<RobotClient> RobotClient::with_channel(std::shared_ptr<ViamChann
 std::shared_ptr<RobotClient> RobotClient::at_address(const std::string& address,
                                                      const Options& options) {
     const char* uri = address.c_str();
-    auto channel = ViamChannel::dial(uri, options.dial_options());
+    auto channel = ViamChannel::dial_initial(uri, options.dial_options());
     std::shared_ptr<RobotClient> robot = RobotClient::with_channel(channel, options);
     robot->should_close_channel_ = true;
 

src/viam/sdk/rpc/dial.cpp

@@ -52,24 +52,35 @@ ViamChannel::ViamChannel(std::shared_ptr<grpc::Channel> channel, const char* pat
 
 DialOptions::DialOptions() = default;
 
-void DialOptions::set_credentials(boost::optional<Credentials> creds) {
+DialOptions& DialOptions::set_credentials(boost::optional<Credentials> creds) {
     credentials_ = std::move(creds);
+
+    return *this;
 }
 
-void DialOptions::set_entity(boost::optional<std::string> entity) {
+DialOptions& DialOptions::set_entity(boost::optional<std::string> entity) {
     auth_entity_ = std::move(entity);
+
+    return *this;
 }
 
-void DialOptions::set_initial_connection_attempts(int attempts) {
+DialOptions& DialOptions::set_initial_connection_attempts(int attempts) {
     initial_connection_attempts_ = attempts;
+
+    return *this;
 }
 
-void DialOptions::set_timeout(std::chrono::duration<float> timeout) {
-    timeout_ = std::move(timeout);
+DialOptions& DialOptions::set_timeout(std::chrono::duration<float> timeout) {
+    timeout_ = timeout;
+
+    return *this;
 }
 
-void DialOptions::set_initial_connection_attempt_timeout(std::chrono::duration<float> timeout) {
-    initial_connection_attempt_timeout_ = std::move(timeout);
+DialOptions& DialOptions::set_initial_connection_attempt_timeout(
+    std::chrono::duration<float> timeout) {
+    initial_connection_attempt_timeout_ = timeout;
+
+    return *this;
 }
 
 const boost::optional<std::string>& DialOptions::entity() const {
@@ -88,25 +99,22 @@ const std::chrono::duration<float>& DialOptions::timeout() const {
     return timeout_;
 }
 
-const std::chrono::duration<float>& DialOptions::initial_connection_attempt_timeout() const {
+std::chrono::duration<float> DialOptions::initial_connection_attempt_timeout() const {
     return initial_connection_attempt_timeout_;
 }
 
-void DialOptions::set_allow_insecure_downgrade(bool allow) {
+DialOptions& DialOptions::set_allow_insecure_downgrade(bool allow) {
     allow_insecure_downgrade_ = allow;
+
+    return *this;
 }
 
 bool DialOptions::allows_insecure_downgrade() const {
     return allow_insecure_downgrade_;
 }
 
-std::shared_ptr<ViamChannel> ViamChannel::dial(const char* uri,
-                                               const boost::optional<DialOptions>& options,
-                                               bool initial_attempt) {
-    if (!initial_attempt) {
-        return dial(std::move(uri), options);
-    }
-
+std::shared_ptr<ViamChannel> ViamChannel::dial_initial(
+    const char* uri, const boost::optional<DialOptions>& options) {
     DialOptions opts = options.get_value_or(DialOptions());
     auto timeout = opts.timeout();
     auto attempts_remaining = opts.initial_connection_attempts();
@@ -120,7 +128,7 @@ std::shared_ptr<ViamChannel> ViamChannel::dial(const char* uri,
             auto connection = dial(uri, opts);
             opts.set_timeout(timeout);
             return connection;
-        } catch (...) {
+        } catch (const std::exception&) {
             attempts_remaining -= 1;
         }
     }

src/viam/sdk/rpc/dial.hpp

@@ -20,11 +20,10 @@ class ViamChannel {
     static std::shared_ptr<ViamChannel> dial(const char* uri,
                                              const boost::optional<DialOptions>& options);
 
-    // allows for specifying that this dial is an initial connection attempt, indicating that
-    // initial connection options should be used.
-    static std::shared_ptr<ViamChannel> dial(const char* uri,
-                                             const boost::optional<DialOptions>& options,
-                                             bool initial_attempt);
+    // Specifies that this dial is an initial connection attempt and that initial connection options
+    // should be used.
+    static std::shared_ptr<ViamChannel> dial_initial(const char* uri,
+                                                     const boost::optional<DialOptions>& options);
 
     const std::shared_ptr<GrpcChannel>& channel() const;
 
@@ -57,14 +56,14 @@ class DialOptions {
     bool allows_insecure_downgrade() const;
     const std::chrono::duration<float>& timeout() const;
     int initial_connection_attempts() const;
-    const std::chrono::duration<float>& initial_connection_attempt_timeout() const;
-
-    void set_entity(boost::optional<std::string> entity);
-    void set_credentials(boost::optional<Credentials> creds);
-    void set_allow_insecure_downgrade(bool allow);
-    void set_timeout(std::chrono::duration<float> timeout);
-    void set_initial_connection_attempts(int attempts);
-    void set_initial_connection_attempt_timeout(std::chrono::duration<float> timeout);
+    std::chrono::duration<float> initial_connection_attempt_timeout() const;
+
+    DialOptions& set_entity(boost::optional<std::string> entity);
+    DialOptions& set_credentials(boost::optional<Credentials> creds);
+    DialOptions& set_allow_insecure_downgrade(bool allow);
+    DialOptions& set_timeout(std::chrono::duration<float> timeout);
+    DialOptions& set_initial_connection_attempts(int attempts);
+    DialOptions& set_initial_connection_attempt_timeout(std::chrono::duration<float> timeout);
 
    private:
     // TODO (RSDK-917): We currently don't provide a flag for disabling webRTC, instead relying on a