Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RSDK-9556 Forward declarations for grpc Client stuff #341

Merged
merged 26 commits into from
Dec 23, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
6bd64aa
update response metadata and timestamp/duration
lia-viam Dec 11, 2024
c245126
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into misc…
lia-viam Dec 11, 2024
7c76e43
insulate grpc client context
lia-viam Dec 11, 2024
0729e87
clean up resource_manager includes
lia-viam Dec 11, 2024
73aa00d
name to/from
lia-viam Dec 12, 2024
75d6d3a
class to struct
lia-viam Dec 13, 2024
1d26302
insulate resource and utils
lia-viam Dec 13, 2024
2fa362f
symmetric ws
lia-viam Dec 13, 2024
924bd86
remove spurious ret
lia-viam Dec 13, 2024
ecf734b
formatting and defaulted ctors
lia-viam Dec 13, 2024
c768df8
de-insulate grpc client context
lia-viam Dec 16, 2024
6a2710f
Merge branch 'main' into misc-proto-update
lia-viam Dec 16, 2024
b18f477
put grpc includes back
lia-viam Dec 17, 2024
a20fb42
generate header for grpc client fwd
lia-viam Dec 17, 2024
c223260
use add
lia-viam Dec 17, 2024
557eefe
Merge branch 'misc-proto-update' into client-fwd
lia-viam Dec 17, 2024
6978fdf
use explicit ctor/assign
lia-viam Dec 17, 2024
f59997f
initialize unique_ptr
lia-viam Dec 17, 2024
7d26e94
Merge branch 'misc-proto-update' into client-fwd
lia-viam Dec 17, 2024
772efcb
client context out of utils
lia-viam Dec 17, 2024
fc38fff
commit configure input file
lia-viam Dec 18, 2024
de2c1ef
Merge branch 'main' of github.com:viamrobotics/viam-cpp-sdk into clie…
lia-viam Dec 18, 2024
0262a08
use the typedef
lia-viam Dec 18, 2024
77f060c
use client reader fwd
lia-viam Dec 18, 2024
66b3b30
fix installation of generated file
lia-viam Dec 23, 2024
aa531ec
Merge branch 'main' into client-fwd
lia-viam Dec 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
5 changes: 4 additions & 1 deletion src/viam/sdk/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}/common/grpc_client_fwd.hpp
)

set_target_properties(
Expand All @@ -207,7 +211,6 @@ target_include_directories(viamsdk
"$<INSTALL_INTERFACE:${VIAMCPPSDK_GRPC_INCLUDE_DIRS}>"
"$<BUILD_INTERFACE:${VIAMCPPSDK_PROTOBUF_INCLUDE_DIRS}>"
"$<INSTALL_INTERFACE:${VIAMCPPSDK_PROTOBUF_INCLUDE_DIRS}>"
PRIVATE
"$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}/../..>"
)

Expand Down
36 changes: 36 additions & 0 deletions src/viam/sdk/common/client_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,12 @@

#include <cstdlib>

#include <grpcpp/client_context.h>

#include <boost/log/trivial.hpp>

#include <viam/sdk/common/private/version_metadata.hpp>

namespace viam {
namespace sdk {

Expand All @@ -16,5 +20,37 @@ namespace client_helper_details {
}

} // namespace client_helper_details

ClientContext::ClientContext() : wrapped_context_(std::make_unique<GrpcClientContext>()) {
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
34 changes: 27 additions & 7 deletions src/viam/sdk/common/client_helper.hpp
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
#pragma once

#include <grpcpp/client_context.h>
#include <grpcpp/support/sync_stream.h>

#include <viam/sdk/common/exception.hpp>
#include <viam/sdk/common/grpc_client_fwd.hpp>
#include <viam/sdk/common/private/utils.hpp>
#include <viam/sdk/common/proto_value.hpp>
#include <viam/sdk/common/utils.hpp>

namespace grpc {

Expand All @@ -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<GrpcClientContext> wrapped_context_;
};

// Method type for a gRPC call that returns a response message type.
template <typename StubType, typename RequestType, typename ResponseType>
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 <typename StubType, typename RequestType, typename ResponseType>
using StreamingMethodType = std::unique_ptr<::grpc::ClientReaderInterface<ResponseType>> (
StubType::*)(::grpc::ClientContext*, const RequestType&);
using StreamingMethodType = std::unique_ptr<GrpcClientReaderInterface<ResponseType>> (StubType::*)(
GrpcClientContext*, const RequestType&);

template <typename ClientType,
typename StubType,
Expand Down
55 changes: 55 additions & 0 deletions src/viam/sdk/common/grpc_client_fwd.hpp.in
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
#pragma once

#cmakedefine VIAMCPPSDK_GRPCXX_LEGACY_CLIENT_FWD

#ifndef VIAMCPPSDK_GRPCXX_LEGACY_CLIENT_FWD

// Forward declaration file for grpc ClientContext and ClientReaderInterface<T>.
// This file provides includes for recent (>= 1.32.0) versions of grpc.

namespace grpc {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(q) why is the namespace grpc here but grpc_impl in the else case below?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In recent versions of grpc the classes are defined in namespace grpc, but in the oldest two versions we support the class definitions are in namespace grpc_impl and typedef'd into namespace grpc, which is incompatible with forward declaring the classes in namespace grpc


class ClientContext;

template <typename>
class ClientReaderInterface;

} // namespace grpc

namespace viam {
namespace sdk {

using GrpcClientContext = ::grpc::ClientContext;

template <typename T>
using GrpcClientReaderInterface = ::grpc::ClientReaderInterface<T>;

} // namespace sdk
} // namespace viam

#else

// Forward declaration file for grpc ClientContext and ClientReaderInterface<T>.
// This file provides includes for the oldest supported versions of grpc (< 1.32.0).

namespace grpc_impl {

class ClientContext;

template <typename>
class ClientReaderInterface;

} // namespace grpc_impl

namespace viam {
namespace sdk {

using GrpcClientContext = ::grpc_impl::ClientContext;

template <typename T>
using GrpcClientReaderInterface = ::grpc_impl::ClientReaderInterface<T>;

} // namespace sdk
} // namespace viam

#endif
32 changes: 0 additions & 32 deletions src/viam/sdk/common/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <viam/api/common/v1/common.pb.h>

#include <viam/sdk/common/private/utils.hpp>
#include <viam/sdk/common/private/version_metadata.hpp>
#include <viam/sdk/components/component.hpp>
#include <viam/sdk/registry/registry.hpp>

Expand Down Expand Up @@ -151,37 +150,6 @@ ProtoStruct with_debug_entry(ProtoStruct&& map) {
return map;
}

ClientContext::ClientContext() : wrapped_context_(std::make_unique<grpc::ClientContext>()) {
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()) {
Expand Down
24 changes: 0 additions & 24 deletions src/viam/sdk/common/utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
#include <memory>

#include <boost/optional/optional.hpp>
#include <grpcpp/client_context.h>

#include <viam/sdk/common/proto_value.hpp>
#include <viam/sdk/components/component.hpp>
Expand Down Expand Up @@ -82,29 +81,6 @@ struct from_proto<common::v1::ResponseMetadata> {
std::vector<unsigned char> string_to_bytes(std::string const& s);
std::string bytes_to_string(std::vector<unsigned char> 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<grpc::ClientContext> wrapped_context_;
};

/// @brief Given a fully qualified resource name, returns remote name (or "" if no remote name
/// exists) and short name
std::pair<std::string, std::string> long_name_to_remote_and_short(const std::string& long_name);
Expand Down
1 change: 1 addition & 0 deletions src/viam/sdk/robot/client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#include <viam/api/robot/v1/robot.grpc.pb.h>
#include <viam/api/robot/v1/robot.pb.h>

#include <viam/sdk/common/client_helper.hpp>
#include <viam/sdk/common/private/repeated_ptr_convert.hpp>
#include <viam/sdk/common/proto_value.hpp>
#include <viam/sdk/common/utils.hpp>
Expand Down
2 changes: 1 addition & 1 deletion src/viam/sdk/services/private/mlmodel_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

#include <grpcpp/channel.h>

#include <viam/sdk/common/utils.hpp>
#include <viam/sdk/common/client_helper.hpp>
#include <viam/sdk/services/private/mlmodel.hpp>

#include <viam/sdk/common/exception.hpp>
Expand Down
Loading