use ifdef for direct dial

Author: lia-viam

CMakeLists.txt

@@ -457,6 +457,8 @@ endif()
 
 if (VIAMCPPSDK_GRPCXX_VERSION VERSION_LESS 1.32.0)
   set(VIAMCPPSDK_GRPCXX_LEGACY_FWD 1)
+elseif(VIAMCPPSDK_GRPCXX_VERSION VERSION_LESS 1.43.0)
+  set(VIAMCPPSDK_GRPCXX_NO_DIRECT_DIAL 1)
 endif()
 
 include(FetchContent)

src/viam/sdk/common/grpc_fwd.hpp.in

@@ -2,6 +2,16 @@
 
 #cmakedefine VIAMCPPSDK_GRPCXX_LEGACY_FWD
 
+// Preprocessor definition for direct dial support in grpc.
+// For grpc >= 1.43.0 it is possible to dial directly over grpc. For older versions,
+// `VIAMCPPSDK_GRPCXX_NO_DIRECT_DIAL` is defined.
+// The default behavior is for the variable to be undefined, providing the behavior for recent
+// versions. If you are experiencing compilation errors in an installed version of the SDK it may be
+// due to a mismatch with the configured version of this header and the grpc version found by the
+// compiler. You may wish to comment/uncomment the define above as needed, or add the definition
+// with `-D` to the compiler.
+#cmakedefine VIAMCPPSDK_GRPCXX_NO_DIRECT_DIAL
+
 // Forward declaration file grpc client and server types.
 // This file provides includes for recent (>= 1.32.0) versions of grpc or older
 // versions, depending on if `VIAMCPPSDK_GRPCXX_LEGACY_FWD` is defined.

src/viam/sdk/rpc/dial.cpp

@@ -10,6 +10,7 @@
 #include <grpc/grpc.h>
 #include <grpcpp/channel.h>
 #include <grpcpp/create_channel.h>
+#include <grpcpp/grpcpp.h>
 #include <grpcpp/security/credentials.h>
 
 #include <viam/api/proto/rpc/v1/auth.grpc.pb.h>
@@ -104,8 +105,15 @@ ViamChannel ViamChannel::dial_initial(const char* uri,
 ViamChannel ViamChannel::dial(const char* uri, const boost::optional<DialOptions>& options) {
     const DialOptions opts = options.get_value_or(DialOptions());
 
+    // If this flag is passed, try to dial directly through grpc if possible.
+    // If grpc is too old to do a direct dial, we fall back to the rust version below even if
+    // disable_webrtc was pased. This is consistent with a hoped-for future semantic where rust
+    // would get the disable_webrtc flag as well rather than deducing it from the URI format, which
+    // is what it currently does.
     if (opts.disable_webrtc) {
+#ifndef VIAMCPPSDK_GRPCXX_NO_DIRECT_DIAL
         return dial_direct(uri, opts);
+#endif
     }
 
     const std::chrono::duration<float> float_timeout = opts.timeout;
@@ -140,7 +148,11 @@ ViamChannel ViamChannel::dial(const char* uri, const boost::optional<DialOptions
         ptr);
 }
 
-void ViamChannel::set_bearer_token(const char* uri, const DialOptions& opts) {
+ViamChannel ViamChannel::dial_direct(const char* uri, const DialOptions& opts) {
+#ifndef VIAMCPPSDK_GRPCXX_NO_DIRECT_DIAL
+    // TODO: if we ever drop older grpc support the logic below might make sense as a
+    // set_bearer_token helper function, but for now it just proliferates the ifdef messiness so
+    // we'll leave it inline
     auto channel_for_auth = sdk::impl::create_viam_auth_channel(uri);
     using namespace proto::rpc::v1;
 
@@ -164,22 +176,15 @@ void ViamChannel::set_bearer_token(const char* uri, const DialOptions& opts) {
 
     Instance::current(Instance::Creation::open_existing).impl_->direct_dial_token =
         resp.access_token();
-}
-
-ViamChannel ViamChannel::dial_direct(const char* uri, const DialOptions& opts) {
-    ViamChannel::set_bearer_token(uri, opts);
-
-#ifndef VIAMCPPSDK_GRPCXX_LEGACY_FWD
-    namespace grpc_experimental = grpc::experimental;
 
-#else
-    namespace grpc_experimental = grpc_impl::experimental;
-#endif
-
-    grpc_experimental::TlsChannelCredentialsOptions c_opts;
+    grpc::experimental::TlsChannelCredentialsOptions c_opts;
     c_opts.set_check_call_host(false);
-    auto creds = grpc_experimental::TlsCredentials(c_opts);
+    auto creds = grpc::experimental::TlsCredentials(c_opts);
     return ViamChannel(sdk::impl::create_viam_grpc_channel(uri, creds));
+#else
+    throw std::logic_error("Tried to call dial_direct on unsupported grpc version " +
+                           grpc::Version());
+#endif
 }
 
 const std::shared_ptr<grpc::Channel>& ViamChannel::channel() const {

src/viam/sdk/rpc/dial.hpp

@@ -50,9 +50,6 @@ class ViamChannel {
    private:
     struct impl;
 
-    // Sets the global bearer token used in client requests for direct dialing over gRPC.
-    static void set_bearer_token(const char* uri, const DialOptions& opts);
-
     static ViamChannel dial_direct(const char* uri, const DialOptions& opts);
 
     std::shared_ptr<GrpcChannel> channel_;

src/viam/sdk/rpc/private/viam_grpc_channel.cpp

@@ -9,13 +9,15 @@ namespace viam {
 namespace sdk {
 namespace impl {
 
+#ifndef VIAMCPPSDK_GRPCXX_NO_DIRECT_DIAL
 std::shared_ptr<grpc::Channel> create_viam_auth_channel(const std::string& address) {
     grpc::experimental::TlsChannelCredentialsOptions opts;
     opts.set_check_call_host(false);
     auto tls_creds = grpc::experimental::TlsCredentials(opts);
 
     return grpc::CreateChannel(address, tls_creds);
 }
+#endif
 
 std::shared_ptr<grpc::Channel> create_viam_grpc_channel(
     const grpc::string& target, const std::shared_ptr<grpc::ChannelCredentials>& credentials) {

src/viam/sdk/rpc/private/viam_grpc_channel.hpp

@@ -14,9 +14,11 @@ namespace impl {
 std::shared_ptr<grpc::Channel> create_viam_grpc_channel(
     const grpc::string& target, const std::shared_ptr<grpc::ChannelCredentials>& credentials);
 
+#ifndef VIAMCPPSDK_GRPCXX_NO_DIRECT_DIAL
 /// @brief Like grpc::CreateChannel, but for the express purpose of returning a channel for making
 /// an AuthService request.
 std::shared_ptr<grpc::Channel> create_viam_auth_channel(const std::string& address);
+#endif
 
 }  // namespace impl
 }  // namespace sdk