From 80efe0d5be02572578f84421f8581833058be582 Mon Sep 17 00:00:00 2001 From: lia <167905060+lia-viam@users.noreply.github.com> Date: Mon, 23 Dec 2024 13:11:00 -0500 Subject: [PATCH] RSDK-9556 Forward declarations for grpc Client stuff (#341) --- CMakeLists.txt | 4 ++ src/viam/sdk/CMakeLists.txt | 5 +- src/viam/sdk/common/client_helper.cpp | 36 ++++++++++++ src/viam/sdk/common/client_helper.hpp | 34 +++++++++--- src/viam/sdk/common/grpc_client_fwd.hpp.in | 55 +++++++++++++++++++ src/viam/sdk/common/utils.cpp | 32 ----------- src/viam/sdk/common/utils.hpp | 24 -------- src/viam/sdk/robot/client.cpp | 1 + .../sdk/services/private/mlmodel_client.cpp | 2 +- 9 files changed, 128 insertions(+), 65 deletions(-) create mode 100644 src/viam/sdk/common/grpc_client_fwd.hpp.in diff --git a/CMakeLists.txt b/CMakeLists.txt index 78d0e9e05..b48d3fc25 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -430,6 +430,10 @@ if ((VIAMCPPSDK_GRPCXX_VERSION VERSION_GREATER 1.51.1) AND (VIAMCPPSDK_PROTOC_VE set(VIAMCPPSDK_BUF_REMOTE_PLUGIN_SUPPORTED 1) endif() +if (VIAMCPPSDK_GRPCXX_VERSION VERSION_LESS 1.32.0) + set(VIAMCPPSDK_GRPCXX_LEGACY_CLIENT_FWD 1) +endif() + include(FetchContent) FetchContent_Declare( diff --git a/src/viam/sdk/CMakeLists.txt b/src/viam/sdk/CMakeLists.txt index fafeb316b..fe5982e1f 100644 --- a/src/viam/sdk/CMakeLists.txt +++ b/src/viam/sdk/CMakeLists.txt @@ -29,6 +29,8 @@ endif() message(WARNING "api proto label is ${VIAMCPPSDK_API_PROTO_LABEL}") configure_file(common/private/version_metadata.hpp.in common/private/version_metadata.hpp @ONLY) +# Configure the grpc client forward declarations file +configure_file(common/grpc_client_fwd.hpp.in common/grpc_client_fwd.hpp) # Set compile and link options based on arguments if (VIAMCPPSDK_USE_WALL_WERROR) @@ -137,6 +139,7 @@ target_sources(viamsdk PUBLIC FILE_SET viamsdk_public_includes TYPE HEADERS BASE_DIRS ../.. + ${CMAKE_CURRENT_BINARY_DIR}/../.. FILES ../../viam/sdk/common/client_helper.hpp ../../viam/sdk/common/exception.hpp @@ -189,6 +192,7 @@ target_sources(viamsdk ../../viam/sdk/spatialmath/geometry.hpp ../../viam/sdk/spatialmath/orientation.hpp ../../viam/sdk/spatialmath/orientation_types.hpp + ${CMAKE_CURRENT_BINARY_DIR}/../../viam/sdk/common/grpc_client_fwd.hpp ) set_target_properties( @@ -207,7 +211,6 @@ target_include_directories(viamsdk "$" "$" "$" - PRIVATE "$" ) diff --git a/src/viam/sdk/common/client_helper.cpp b/src/viam/sdk/common/client_helper.cpp index 84990153d..1d98249b6 100644 --- a/src/viam/sdk/common/client_helper.cpp +++ b/src/viam/sdk/common/client_helper.cpp @@ -2,8 +2,12 @@ #include +#include + #include +#include + namespace viam { namespace sdk { @@ -16,5 +20,37 @@ namespace client_helper_details { } } // namespace client_helper_details + +ClientContext::ClientContext() : wrapped_context_(std::make_unique()) { + set_client_ctx_authority_(); + add_viam_client_version_(); +} + +ClientContext::~ClientContext() = default; + +ClientContext::operator const GrpcClientContext*() const { + return wrapped_context_.get(); +} + +ClientContext::operator GrpcClientContext*() { + return wrapped_context_.get(); +} + +void ClientContext::try_cancel() { + wrapped_context_->TryCancel(); +} + +void ClientContext::set_debug_key(const std::string& debug_key) { + wrapped_context_->AddMetadata("dtname", debug_key); +} + +void ClientContext::set_client_ctx_authority_() { + wrapped_context_->set_authority("viam-placeholder"); +} + +void ClientContext::add_viam_client_version_() { + wrapped_context_->AddMetadata("viam_client", impl::k_version); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/common/client_helper.hpp b/src/viam/sdk/common/client_helper.hpp index c68e7fe85..27171c9e0 100644 --- a/src/viam/sdk/common/client_helper.hpp +++ b/src/viam/sdk/common/client_helper.hpp @@ -1,12 +1,9 @@ #pragma once -#include -#include - #include +#include #include #include -#include namespace grpc { @@ -23,16 +20,39 @@ namespace client_helper_details { } // namespace client_helper_details +// the authority on a grpc::ClientContext is sometimes set to an invalid uri on mac, causing +// `rust-utils` to fail to process gRPC requests. This class provides a convenience wrapper around a +// grpc ClientContext that allows us to make any necessary modifications to authority or else where +// to avoid runtime issues. +// For more details, see https://viam.atlassian.net/browse/RSDK-5194. +class ClientContext { + public: + ClientContext(); + ~ClientContext(); + + void try_cancel(); + + operator GrpcClientContext*(); + operator const GrpcClientContext*() const; + + void set_debug_key(const std::string& debug_key); + + private: + void set_client_ctx_authority_(); + void add_viam_client_version_(); + std::unique_ptr wrapped_context_; +}; + // Method type for a gRPC call that returns a response message type. template -using SyncMethodType = ::grpc::Status (StubType::*)(::grpc::ClientContext*, +using SyncMethodType = ::grpc::Status (StubType::*)(GrpcClientContext*, const RequestType&, ResponseType*); // Method type for a gRPC call that returns a stream of response message type. template -using StreamingMethodType = std::unique_ptr<::grpc::ClientReaderInterface> ( - StubType::*)(::grpc::ClientContext*, const RequestType&); +using StreamingMethodType = std::unique_ptr> (StubType::*)( + GrpcClientContext*, const RequestType&); template . +// This file provides includes for recent (>= 1.32.0) versions of grpc. + +namespace grpc { + +class ClientContext; + +template +class ClientReaderInterface; + +} // namespace grpc + +namespace viam { +namespace sdk { + +using GrpcClientContext = ::grpc::ClientContext; + +template +using GrpcClientReaderInterface = ::grpc::ClientReaderInterface; + +} // namespace sdk +} // namespace viam + +#else + +// Forward declaration file for grpc ClientContext and ClientReaderInterface. +// This file provides includes for the oldest supported versions of grpc (< 1.32.0). + +namespace grpc_impl { + +class ClientContext; + +template +class ClientReaderInterface; + +} // namespace grpc_impl + +namespace viam { +namespace sdk { + +using GrpcClientContext = ::grpc_impl::ClientContext; + +template +using GrpcClientReaderInterface = ::grpc_impl::ClientReaderInterface; + +} // namespace sdk +} // namespace viam + +#endif diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index fdd03b6da..d6375cbde 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -18,7 +18,6 @@ #include #include -#include #include #include @@ -151,37 +150,6 @@ ProtoStruct with_debug_entry(ProtoStruct&& map) { return map; } -ClientContext::ClientContext() : wrapped_context_(std::make_unique()) { - set_client_ctx_authority_(); - add_viam_client_version_(); -} - -ClientContext::~ClientContext() = default; - -ClientContext::operator const grpc::ClientContext*() const { - return wrapped_context_.get(); -} - -ClientContext::operator grpc::ClientContext*() { - return wrapped_context_.get(); -} - -void ClientContext::try_cancel() { - wrapped_context_->TryCancel(); -} - -void ClientContext::set_debug_key(const std::string& debug_key) { - wrapped_context_->AddMetadata("dtname", debug_key); -} - -void ClientContext::set_client_ctx_authority_() { - wrapped_context_->set_authority("viam-placeholder"); -} - -void ClientContext::add_viam_client_version_() { - wrapped_context_->AddMetadata("viam_client", impl::k_version); -} - bool from_dm_from_extra(const ProtoStruct& extra) { auto pos = extra.find("fromDataManagement"); if (pos != extra.end()) { diff --git a/src/viam/sdk/common/utils.hpp b/src/viam/sdk/common/utils.hpp index cb3c76532..f47442750 100644 --- a/src/viam/sdk/common/utils.hpp +++ b/src/viam/sdk/common/utils.hpp @@ -3,7 +3,6 @@ #include #include -#include #include #include @@ -82,29 +81,6 @@ struct from_proto { std::vector string_to_bytes(std::string const& s); std::string bytes_to_string(std::vector const& b); -// the authority on a grpc::ClientContext is sometimes set to an invalid uri on mac, causing -// `rust-utils` to fail to process gRPC requests. This class provides a convenience wrapper around a -// grpc ClientContext that allows us to make any necessary modifications to authority or else where -// to avoid runtime issues. -// For more details, see https://viam.atlassian.net/browse/RSDK-5194. -class ClientContext { - public: - ClientContext(); - ~ClientContext(); - - void try_cancel(); - - operator grpc::ClientContext*(); - operator const grpc::ClientContext*() const; - - void set_debug_key(const std::string& debug_key); - - private: - void set_client_ctx_authority_(); - void add_viam_client_version_(); - std::unique_ptr wrapped_context_; -}; - /// @brief Given a fully qualified resource name, returns remote name (or "" if no remote name /// exists) and short name std::pair long_name_to_remote_and_short(const std::string& long_name); diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index 3fe5d4d1f..f0befb2d9 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -19,6 +19,7 @@ #include #include +#include #include #include #include diff --git a/src/viam/sdk/services/private/mlmodel_client.cpp b/src/viam/sdk/services/private/mlmodel_client.cpp index 8b7fd6039..2c98960dd 100644 --- a/src/viam/sdk/services/private/mlmodel_client.cpp +++ b/src/viam/sdk/services/private/mlmodel_client.cpp @@ -16,7 +16,7 @@ #include -#include +#include #include #include