From 6bd64aad1b60a1750fafc41f5aab3bbef42fbc1e Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 11 Dec 2024 12:31:22 -0500 Subject: [PATCH 01/20] update response metadata and timestamp/duration --- src/viam/sdk/common/utils.cpp | 87 +++++++++---------- src/viam/sdk/common/utils.hpp | 61 ++++++++++--- .../sdk/components/private/board_client.cpp | 2 +- .../sdk/components/private/board_server.cpp | 2 +- .../sdk/components/private/camera_client.cpp | 2 +- .../sdk/components/private/camera_server.cpp | 2 +- src/viam/sdk/robot/client.cpp | 4 +- .../sdk/services/private/motion_client.cpp | 2 +- .../sdk/services/private/motion_server.cpp | 2 +- src/viam/sdk/tests/test_common.cpp | 24 ++--- 10 files changed, 114 insertions(+), 74 deletions(-) diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index 098188ba9..2942187d1 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -22,52 +22,44 @@ namespace viam { namespace sdk { -std::vector string_to_bytes(const std::string& s) { - std::vector bytes(s.begin(), s.end()); - return bytes; -}; - -std::string bytes_to_string(const std::vector& b) { - std::string img_string(b.begin(), b.end()); - return img_string; -}; - -time_pt timestamp_to_time_pt(const google::protobuf::Timestamp& timestamp) { - return time_pt{std::chrono::seconds{timestamp.seconds()} + - std::chrono::nanoseconds{timestamp.nanos()}}; +bool operator==(const response_metadata& lhs, const response_metadata& rhs) { + return lhs.captured_at == rhs.captured_at; } -google::protobuf::Timestamp time_pt_to_timestamp(time_pt tp) { +namespace proto_convert_details { + +void to_proto::operator()(time_pt tp, google::protobuf::Timestamp* result) const { const std::chrono::nanoseconds since_epoch = tp.time_since_epoch(); const auto sec_floor = std::chrono::duration_cast(since_epoch); const std::chrono::nanoseconds nano_part = since_epoch - sec_floor; - google::protobuf::Timestamp result; - - result.set_seconds(sec_floor.count()); - result.set_nanos(static_cast(nano_part.count())); - - return result; + result->set_seconds(sec_floor.count()); + result->set_nanos(static_cast(nano_part.count())); } -response_metadata response_metadata::from_proto(const viam::common::v1::ResponseMetadata& proto) { - response_metadata metadata; - metadata.captured_at = timestamp_to_time_pt(proto.captured_at()); - return metadata; +time_pt from_proto::operator()( + const google::protobuf::Timestamp* timestamp) const { + return time_pt{std::chrono::seconds{timestamp->seconds()} + + std::chrono::nanoseconds{timestamp->nanos()}}; } -viam::common::v1::ResponseMetadata response_metadata::to_proto(const response_metadata& metadata) { - viam::common::v1::ResponseMetadata proto; - google::protobuf::Timestamp ts = time_pt_to_timestamp(metadata.captured_at); - *proto.mutable_captured_at() = std::move(ts); - return proto; +void to_proto::operator()(std::chrono::microseconds duration, + google::protobuf::Duration* proto) const { + namespace sc = std::chrono; + + const sc::seconds seconds = sc::duration_cast(duration); + const sc::nanoseconds nanos = duration - seconds; + + proto->set_nanos(static_cast(nanos.count())); + proto->set_seconds(seconds.count()); } -std::chrono::microseconds from_proto(const google::protobuf::Duration& proto) { +std::chrono::microseconds from_proto::operator()( + const google::protobuf::Duration* proto) const { namespace sc = std::chrono; - const sc::seconds seconds_part{proto.seconds()}; - const sc::nanoseconds nanos_part{proto.nanos()}; + const sc::seconds seconds_part{proto->seconds()}; + const sc::nanoseconds nanos_part{proto->nanos()}; const sc::microseconds from_seconds = sc::duration_cast(seconds_part); sc::microseconds from_nanos = sc::duration_cast(nanos_part); @@ -78,18 +70,29 @@ std::chrono::microseconds from_proto(const google::protobuf::Duration& proto) { from_nanos += sc::microseconds(1); } return from_seconds + from_nanos; + return std::chrono::microseconds(); } -google::protobuf::Duration to_proto(std::chrono::microseconds duration) { - namespace sc = std::chrono; +void to_proto::operator()(const response_metadata& self, + common::v1::ResponseMetadata* proto) const { + *(proto->mutable_captured_at()) = v2::to_proto(self.captured_at); +} - const sc::seconds seconds = sc::duration_cast(duration); - const sc::nanoseconds nanos = duration - seconds; +response_metadata from_proto::operator()( + const common::v1::ResponseMetadata* proto) const { + return {v2::from_proto(proto->captured_at())}; +} + +} // namespace proto_convert_details + +std::vector string_to_bytes(const std::string& s) { + std::vector bytes(s.begin(), s.end()); + return bytes; +} - google::protobuf::Duration proto; - proto.set_nanos(static_cast(nanos.count())); - proto.set_seconds(seconds.count()); - return proto; +std::string bytes_to_string(const std::vector& b) { + std::string img_string(b.begin(), b.end()); + return img_string; } void set_logger_severity_from_args(int argc, char** argv) { @@ -101,10 +104,6 @@ void set_logger_severity_from_args(int argc, char** argv) { boost::log::core::get()->set_filter(boost::log::trivial::severity >= boost::log::trivial::info); } -bool operator==(const response_metadata& lhs, const response_metadata& rhs) { - return lhs.captured_at == rhs.captured_at; -} - void ClientContext::set_client_ctx_authority_() { wrapped_context_.set_authority("viam-placeholder"); } diff --git a/src/viam/sdk/common/utils.hpp b/src/viam/sdk/common/utils.hpp index 41fc6b91a..e69885468 100644 --- a/src/viam/sdk/common/utils.hpp +++ b/src/viam/sdk/common/utils.hpp @@ -9,6 +9,25 @@ #include #include +namespace google { +namespace protobuf { + +class Duration; +class Timestamp; + +} // namespace protobuf +} // namespace google + +namespace viam { +namespace common { +namespace v1 { + +class ResponseMetadata; + +} +} // namespace common +} // namespace viam + namespace viam { namespace sdk { @@ -22,25 +41,47 @@ using time_pt = std::chrono::time_point +struct to_proto { + void operator()(time_pt, google::protobuf::Timestamp*) const; +}; + +template <> +struct from_proto { + time_pt operator()(const google::protobuf::Timestamp*) const; +}; + +template <> +struct to_proto { + void operator()(std::chrono::microseconds, google::protobuf::Duration*) const; +}; -/// @brief convert a time_pt to a google::protobuf::Timestamp. -google::protobuf::Timestamp time_pt_to_timestamp(time_pt); +template <> +struct from_proto { + std::chrono::microseconds operator()(const google::protobuf::Duration*) const; +}; + +template <> +struct to_proto { + void operator()(const response_metadata&, common::v1::ResponseMetadata*) const; +}; + +template <> +struct from_proto { + response_metadata operator()(const common::v1::ResponseMetadata*) const; +}; + +} // namespace proto_convert_details std::vector string_to_bytes(std::string const& s); std::string bytes_to_string(std::vector const& b); -std::chrono::microseconds from_proto(const google::protobuf::Duration& proto); -google::protobuf::Duration to_proto(std::chrono::microseconds duration); - // 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 diff --git a/src/viam/sdk/components/private/board_client.cpp b/src/viam/sdk/components/private/board_client.cpp index b6c6a952f..11af0428a 100644 --- a/src/viam/sdk/components/private/board_client.cpp +++ b/src/viam/sdk/components/private/board_client.cpp @@ -182,7 +182,7 @@ void BoardClient::set_power_mode(power_mode power_mode, [&](auto& request) { request.set_power_mode(to_proto(power_mode)); if (duration.has_value()) { - *request.mutable_duration() = ::viam::sdk::to_proto(duration.get()); + *request.mutable_duration() = v2::to_proto(duration.get()); } }) .invoke(); diff --git a/src/viam/sdk/components/private/board_server.cpp b/src/viam/sdk/components/private/board_server.cpp index b3064912a..9b7338d65 100644 --- a/src/viam/sdk/components/private/board_server.cpp +++ b/src/viam/sdk/components/private/board_server.cpp @@ -226,7 +226,7 @@ ::grpc::Status BoardServer::SetPowerMode( return make_service_helper( "BoardServer::SetPowerMode", this, request)([&](auto& helper, auto& board) { if (request->has_duration()) { - auto duration = ::viam::sdk::from_proto(request->duration()); + auto duration = v2::from_proto(request->duration()); board->set_power_mode(from_proto(request->power_mode()), helper.getExtra(), duration); } else { board->set_power_mode(from_proto(request->power_mode()), helper.getExtra()); diff --git a/src/viam/sdk/components/private/camera_client.cpp b/src/viam/sdk/components/private/camera_client.cpp index eb2e24ced..0818bfb39 100644 --- a/src/viam/sdk/components/private/camera_client.cpp +++ b/src/viam/sdk/components/private/camera_client.cpp @@ -56,7 +56,7 @@ Camera::image_collection from_proto(const viam::component::camera::v1::GetImages images.push_back(raw_image); } image_collection.images = std::move(images); - image_collection.metadata = response_metadata::from_proto(proto.response_metadata()); + image_collection.metadata = v2::from_proto(proto.response_metadata()); return image_collection; } diff --git a/src/viam/sdk/components/private/camera_server.cpp b/src/viam/sdk/components/private/camera_server.cpp index eb3532814..ab4b65179 100644 --- a/src/viam/sdk/components/private/camera_server.cpp +++ b/src/viam/sdk/components/private/camera_server.cpp @@ -94,7 +94,7 @@ ::grpc::Status CameraServer::GetImages( proto_image.set_image(img_string); *response->mutable_images()->Add() = std::move(proto_image); } - *response->mutable_response_metadata() = response_metadata::to_proto(image_coll.metadata); + *response->mutable_response_metadata() = v2::to_proto(image_coll.metadata); }); } diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index 290ea8210..0c784d071 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -89,7 +89,7 @@ RobotClient::status from_proto(const Status& proto) { status.status_map = v2::from_proto(proto.status()); } if (proto.has_last_reconfigured()) { - status.last_reconfigured = timestamp_to_time_pt(proto.last_reconfigured()); + status.last_reconfigured = v2::from_proto(proto.last_reconfigured()); } return status; } @@ -105,7 +105,7 @@ RobotClient::operation from_proto(const Operation& proto) { op.arguments = v2::from_proto(proto.arguments()); } if (proto.has_started()) { - op.started = timestamp_to_time_pt(proto.started()); + op.started = v2::from_proto(proto.started()); } return op; } diff --git a/src/viam/sdk/services/private/motion_client.cpp b/src/viam/sdk/services/private/motion_client.cpp index 115fd5280..e0c686f7d 100644 --- a/src/viam/sdk/services/private/motion_client.cpp +++ b/src/viam/sdk/services/private/motion_client.cpp @@ -109,7 +109,7 @@ Motion::plan_status from_proto(const service::motion::v1::PlanStatus& proto) { if (proto.has_reason()) { mps.reason = proto.reason(); } - mps.timestamp = timestamp_to_time_pt(proto.timestamp()); + mps.timestamp = v2::from_proto(proto.timestamp()); return mps; } diff --git a/src/viam/sdk/services/private/motion_server.cpp b/src/viam/sdk/services/private/motion_server.cpp index a703ba587..1dad3ef57 100644 --- a/src/viam/sdk/services/private/motion_server.cpp +++ b/src/viam/sdk/services/private/motion_server.cpp @@ -39,7 +39,7 @@ service::motion::v1::PlanState to_proto(const Motion::plan_state& state) { service::motion::v1::PlanStatus to_proto(const Motion::plan_status& ps) { service::motion::v1::PlanStatus proto; - *proto.mutable_timestamp() = time_pt_to_timestamp(ps.timestamp); + *proto.mutable_timestamp() = v2::to_proto(ps.timestamp); if (ps.reason) { *proto.mutable_reason() = *ps.reason; } diff --git a/src/viam/sdk/tests/test_common.cpp b/src/viam/sdk/tests/test_common.cpp index 6e4454b03..b804bf0af 100644 --- a/src/viam/sdk/tests/test_common.cpp +++ b/src/viam/sdk/tests/test_common.cpp @@ -24,9 +24,9 @@ BOOST_AUTO_TEST_CASE(test_zero) { Duration input; input.set_nanos(0); input.set_seconds(0); - auto converted = from_proto(input); + auto converted = v2::from_proto(input); BOOST_CHECK_EQUAL(converted.count(), 0); - auto reconverted = to_proto(converted); + auto reconverted = v2::to_proto(converted); BOOST_CHECK_EQUAL(reconverted.nanos(), 0); BOOST_CHECK_EQUAL(reconverted.seconds(), 0); } @@ -35,9 +35,9 @@ BOOST_AUTO_TEST_CASE(test_rounding_negative) { Duration input; input.set_nanos(-100); input.set_seconds(0); - auto converted = from_proto(input); + auto converted = v2::from_proto(input); BOOST_CHECK_EQUAL(converted.count(), -1); - auto reconverted = to_proto(converted); + auto reconverted = v2::to_proto(converted); BOOST_CHECK_EQUAL(reconverted.nanos(), -1000); BOOST_CHECK_EQUAL(reconverted.seconds(), 0); } @@ -46,9 +46,9 @@ BOOST_AUTO_TEST_CASE(test_rounding_positive) { Duration input; input.set_nanos(999); input.set_seconds(0); - auto converted = from_proto(input); + auto converted = v2::from_proto(input); BOOST_CHECK_EQUAL(converted.count(), 1); - auto reconverted = to_proto(converted); + auto reconverted = v2::to_proto(converted); BOOST_CHECK_EQUAL(reconverted.nanos(), 1000); BOOST_CHECK_EQUAL(reconverted.seconds(), 0); } @@ -58,9 +58,9 @@ BOOST_AUTO_TEST_CASE(test_mixed_sign_rounding) { // Should round to -1 μs input.set_nanos(-500); input.set_seconds(1); - auto converted = from_proto(input); + auto converted = v2::from_proto(input); BOOST_CHECK_EQUAL(converted.count(), 1e6 - 1); - auto reconverted = to_proto(converted); + auto reconverted = v2::to_proto(converted); BOOST_CHECK_EQUAL(reconverted.nanos(), 1e9 - 1000); BOOST_CHECK_EQUAL(reconverted.seconds(), 0); } @@ -70,9 +70,9 @@ BOOST_AUTO_TEST_CASE(test_medium_positive) { // Should round to 2 μs input.set_nanos(1500); input.set_seconds(1000); - auto converted = from_proto(input); + auto converted = v2::from_proto(input); BOOST_CHECK_EQUAL(converted.count(), 1000 * 1e6 + 2); - auto reconverted = to_proto(converted); + auto reconverted = v2::to_proto(converted); BOOST_CHECK_EQUAL(reconverted.nanos(), 2000); BOOST_CHECK_EQUAL(reconverted.seconds(), 1000); } @@ -85,9 +85,9 @@ BOOST_AUTO_TEST_CASE(test_large_positive) { // compliant with the proto spec int64_t max_seconds = 10e3 * 365 * 24 * 60 * 60; input.set_seconds(max_seconds); - auto converted = from_proto(input); + auto converted = v2::from_proto(input); BOOST_CHECK_EQUAL(converted.count(), 1e6 * max_seconds + 2); - auto reconverted = to_proto(converted); + auto reconverted = v2::to_proto(converted); BOOST_CHECK_EQUAL(reconverted.nanos(), 2000); BOOST_CHECK_EQUAL(reconverted.seconds(), max_seconds); } From 7c76e43c50f97b5e089454b20fe690257527f001 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:02:17 -0500 Subject: [PATCH 02/20] insulate grpc client context --- src/viam/sdk/common/utils.cpp | 30 ++++++++++++++++-------------- src/viam/sdk/common/utils.hpp | 16 ++++++++++++---- 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index 2942187d1..b4c2d8968 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -104,10 +104,6 @@ void set_logger_severity_from_args(int argc, char** argv) { boost::log::core::get()->set_filter(boost::log::trivial::severity >= boost::log::trivial::info); } -void ClientContext::set_client_ctx_authority_() { - wrapped_context_.set_authority("viam-placeholder"); -} - std::string random_debug_key() { static const char alphanum[] = "abcdefghijklmnopqrstuvwxyz"; static std::default_random_engine generator( @@ -153,25 +149,31 @@ ProtoStruct with_debug_entry(ProtoStruct&& map) { return map; } -void ClientContext::set_debug_key(const std::string& debug_key) { - wrapped_context_.AddMetadata("dtname", debug_key); -} - -void ClientContext::add_viam_client_version_() { - wrapped_context_.AddMetadata("viam_client", impl::k_version); -} - ClientContext::ClientContext() { set_client_ctx_authority_(); add_viam_client_version_(); } +ClientContext::~ClientContext() = default; + ClientContext::operator const grpc::ClientContext*() const { - return &wrapped_context_; + return wrapped_context_.get(); } ClientContext::operator grpc::ClientContext*() { - return &wrapped_context_; + return wrapped_context_.get(); +} + +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) { diff --git a/src/viam/sdk/common/utils.hpp b/src/viam/sdk/common/utils.hpp index e69885468..795521e48 100644 --- a/src/viam/sdk/common/utils.hpp +++ b/src/viam/sdk/common/utils.hpp @@ -1,9 +1,8 @@ #pragma once -#include -#include +#include -#include +#include #include #include @@ -18,6 +17,12 @@ class Timestamp; } // namespace protobuf } // namespace google +namespace grpc { + +class ClientContext; + +} + namespace viam { namespace common { namespace v1 { @@ -90,14 +95,17 @@ std::string bytes_to_string(std::vector const& b); class ClientContext { public: ClientContext(); + ~ClientContext(); + 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_(); - grpc::ClientContext wrapped_context_; + std::unique_ptr wrapped_context_; }; /// @brief Given a fully qualified resource name, returns remote name (or "" if no remote name From 0729e8793935ffd76bce0f3fa6cef4c30c1495d3 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 11 Dec 2024 13:07:24 -0500 Subject: [PATCH 03/20] clean up resource_manager includes --- src/viam/sdk/common/service_helper.hpp | 2 ++ src/viam/sdk/resource/resource_manager.hpp | 5 +---- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/viam/sdk/common/service_helper.hpp b/src/viam/sdk/common/service_helper.hpp index 0dd193fcb..b9435bbb3 100644 --- a/src/viam/sdk/common/service_helper.hpp +++ b/src/viam/sdk/common/service_helper.hpp @@ -2,6 +2,8 @@ #include +#include + #include namespace viam { diff --git a/src/viam/sdk/resource/resource_manager.hpp b/src/viam/sdk/resource/resource_manager.hpp index 3bc894f49..0a8f62c32 100644 --- a/src/viam/sdk/resource/resource_manager.hpp +++ b/src/viam/sdk/resource/resource_manager.hpp @@ -9,9 +9,6 @@ #include #include -#include - -#include #include #include @@ -63,7 +60,7 @@ class ResourceManager { /// @brief Returns a reference to the existing resources within the manager. const std::unordered_map>& resources() const; - ResourceManager(){}; + ResourceManager() {}; private: std::mutex lock_; From 73aa00dba2bfcd587b55819ba6e7dbe01bf34227 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Thu, 12 Dec 2024 13:24:15 -0500 Subject: [PATCH 04/20] name to/from --- src/viam/sdk/config/resource.hpp | 9 ++++ src/viam/sdk/module/handler_map.cpp | 4 +- src/viam/sdk/resource/resource_api.cpp | 47 ++++++++++--------- src/viam/sdk/resource/resource_api.hpp | 42 +++++++++++++---- src/viam/sdk/robot/client.cpp | 4 +- src/viam/sdk/robot/service.cpp | 4 +- .../sdk/services/private/motion_client.cpp | 24 +++++----- .../sdk/services/private/motion_server.cpp | 24 +++++----- src/viam/sdk/tests/test_resource.cpp | 6 +-- 9 files changed, 101 insertions(+), 63 deletions(-) diff --git a/src/viam/sdk/config/resource.hpp b/src/viam/sdk/config/resource.hpp index a89ff0f9e..b1a703a6a 100644 --- a/src/viam/sdk/config/resource.hpp +++ b/src/viam/sdk/config/resource.hpp @@ -24,7 +24,16 @@ class ResourceConfig { public: static ResourceConfig from_proto(const viam::app::v1::ComponentConfig& proto_cfg); viam::app::v1::ComponentConfig to_proto() const; + ResourceConfig(std::string type, + std::string name, + std::string namespace_, + ProtoStruct attributes, + std::string api, + Model model, + LinkConfig frame); + ResourceConfig(std::string type); + Name resource_name(); const API& api() const; const LinkConfig& frame() const; diff --git a/src/viam/sdk/module/handler_map.cpp b/src/viam/sdk/module/handler_map.cpp index d6e5fe0ed..2878c4361 100644 --- a/src/viam/sdk/module/handler_map.cpp +++ b/src/viam/sdk/module/handler_map.cpp @@ -24,7 +24,7 @@ viam::module::v1::HandlerMap HandlerMap_::to_proto() const { } viam::robot::v1::ResourceRPCSubtype rpc_subtype; const Name name(h.first.api(), "", ""); - const viam::common::v1::ResourceName resource_name = name.to_proto(); + const viam::common::v1::ResourceName resource_name = v2::to_proto(name); *rpc_subtype.mutable_subtype() = resource_name; *rpc_subtype.mutable_proto_service() = h.first.proto_service_name(); *hd.mutable_subtype() = rpc_subtype; @@ -34,7 +34,7 @@ viam::module::v1::HandlerMap HandlerMap_::to_proto() const { return proto; }; -HandlerMap_::HandlerMap_(){}; +HandlerMap_::HandlerMap_() {} // NOLINTNEXTLINE(readability-const-return-type) const HandlerMap_ HandlerMap_::from_proto(const viam::module::v1::HandlerMap& proto) { diff --git a/src/viam/sdk/resource/resource_api.cpp b/src/viam/sdk/resource/resource_api.cpp index 3cca16d0e..06eadc248 100644 --- a/src/viam/sdk/resource/resource_api.cpp +++ b/src/viam/sdk/resource/resource_api.cpp @@ -86,6 +86,9 @@ bool API::is_component_type() { return (this->resource_type() == "component"); } +Name::Name(API api, std::string remote, std::string name) + : api_(std::move(api)), remote_name_(std::move(remote)), name_(std::move(name)) {} + const API& Name::api() const { return api_; } @@ -112,26 +115,6 @@ std::string Name::short_name() const { return name_; } -viam::common::v1::ResourceName Name::to_proto() const { - viam::common::v1::ResourceName rn; - *rn.mutable_namespace_() = this->api().type_namespace(); - if (this->remote_name().empty()) { - *rn.mutable_name() = this->name(); - } else { - *rn.mutable_name() = this->remote_name() + ":" + this->name(); - } - *rn.mutable_type() = this->api().resource_type(); - *rn.mutable_subtype() = this->api().resource_subtype(); - return rn; -} - -Name Name::from_proto(const viam::common::v1::ResourceName& proto) { - auto name_parts = long_name_to_remote_and_short(proto.name()); - - return Name( - {proto.namespace_(), proto.type(), proto.subtype()}, name_parts.first, name_parts.second); -}; - Name Name::from_string(std::string name) { if (!std::regex_match(name, NAME_REGEX)) { throw Exception("Received invalid Name string: " + name); @@ -153,8 +136,28 @@ Name Name::from_string(std::string name) { return Name(api, remote, resource_name); } -Name::Name(API api, std::string remote, std::string name) - : api_(std::move(api)), remote_name_(std::move(remote)), name_(std::move(name)) {} +namespace proto_convert_details { + +void to_proto::operator()(const Name& self, common::v1::ResourceName* proto) const { + *proto->mutable_namespace_() = self.api().type_namespace(); + if (self.remote_name().empty()) { + *proto->mutable_name() = self.name(); + } else { + *proto->mutable_name() = self.remote_name() + ":" + self.name(); + } + *proto->mutable_type() = self.api().resource_type(); + *proto->mutable_subtype() = self.api().resource_subtype(); +} + +Name from_proto::operator()(const common::v1::ResourceName* proto) const { + auto name_parts = long_name_to_remote_and_short(proto->name()); + + return Name({proto->namespace_(), proto->type(), proto->subtype()}, + name_parts.first, + name_parts.second); +} + +} // namespace proto_convert_details bool operator==(const API& lhs, const API& rhs) { return lhs.to_string() == rhs.to_string(); diff --git a/src/viam/sdk/resource/resource_api.hpp b/src/viam/sdk/resource/resource_api.hpp index 5677a8fd4..868d14a37 100644 --- a/src/viam/sdk/resource/resource_api.hpp +++ b/src/viam/sdk/resource/resource_api.hpp @@ -4,7 +4,17 @@ #include -#include +#include + +namespace viam { +namespace common { +namespace v1 { + +class ResourceName; + +} +} // namespace common +} // namespace viam namespace viam { namespace sdk { @@ -14,7 +24,7 @@ namespace sdk { class APIType { public: APIType(std::string namespace_, std::string resource_type); - APIType(){}; + APIType() {}; virtual std::string to_string() const; const std::string& type_namespace() const; @@ -33,7 +43,7 @@ class APIType { class API : public APIType { public: virtual std::string to_string() const override; - API(){}; + API() {}; API(std::string namespace_, std::string resource_type, std::string resource_subtype); API(APIType type, std::string resource_subtype); static API from_string(std::string api); @@ -62,16 +72,18 @@ class API : public APIType { /// @brief A name for specific instances of resources. class Name { public: - std::string short_name() const; - std::string to_string() const; - viam::common::v1::ResourceName to_proto() const; - static Name from_proto(const viam::common::v1::ResourceName& proto); static Name from_string(std::string name); + Name(API api, std::string remote_name, std::string name); - Name(){}; + Name() {}; + + std::string short_name() const; + std::string to_string() const; + const API& api() const; const std::string& name() const; const std::string& remote_name() const; + friend bool operator==(const Name& lhs, const Name& rhs); friend std::ostream& operator<<(std::ostream& os, const Name& v); @@ -81,6 +93,20 @@ class Name { std::string name_; }; +namespace proto_convert_details { + +template <> +struct to_proto { + void operator()(const Name&, common::v1::ResourceName*) const; +}; + +template <> +struct from_proto { + Name operator()(const common::v1::ResourceName*) const; +}; + +} // namespace proto_convert_details + class RPCSubtype { public: bool operator<(const RPCSubtype& rhs) const { diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index faec60ab0..3fe5d4d1f 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -203,7 +203,7 @@ void RobotClient::refresh() { std::unordered_map> new_resources; std::vector current_resources; for (const auto& name : resp.resources()) { - current_resources.push_back(Name::from_proto(name)); + current_resources.push_back(v2::from_proto(name)); if (name.subtype() == "remote") { continue; } @@ -404,7 +404,7 @@ void RobotClient::stop_all(const std::unordered_map& extra) { const ProtoStruct& params = xtra.second; const google::protobuf::Struct s = v2::to_proto(params); viam::robot::v1::StopExtraParameters stop; - *stop.mutable_name() = name.to_proto(); + *stop.mutable_name() = v2::to_proto(name); *stop.mutable_params() = s; *ep->Add() = stop; } diff --git a/src/viam/sdk/robot/service.cpp b/src/viam/sdk/robot/service.cpp index 2ebdf2a9a..a841e9e5b 100644 --- a/src/viam/sdk/robot/service.cpp +++ b/src/viam/sdk/robot/service.cpp @@ -66,7 +66,7 @@ std::vector RobotService_::generate_metadata_() { std::vector metadata; for (const auto& key_and_val : resource_manager()->resources()) { for (const Name& name : registered_models_for_resource(key_and_val.second)) { - metadata.push_back(name.to_proto()); + metadata.push_back(v2::to_proto(name)); } } return metadata; @@ -104,7 +104,7 @@ ::grpc::Status RobotService_::StopAll(::grpc::ServerContext*, for (const auto& r : resource_manager()->resources()) { const std::shared_ptr resource = r.second; - const ResourceName rn = resource->get_resource_name().to_proto(); + const ResourceName rn = v2::to_proto(resource->get_resource_name()); const std::string rn_ = rn.SerializeAsString(); if (extra.find(rn_) != extra.end()) { try { diff --git a/src/viam/sdk/services/private/motion_client.cpp b/src/viam/sdk/services/private/motion_client.cpp index e0c686f7d..9252e9c60 100644 --- a/src/viam/sdk/services/private/motion_client.cpp +++ b/src/viam/sdk/services/private/motion_client.cpp @@ -19,8 +19,8 @@ namespace impl { service::motion::v1::ObstacleDetector to_proto(const obstacle_detector& od) { service::motion::v1::ObstacleDetector proto; - *proto.mutable_vision_service() = od.vision_service.to_proto(); - *proto.mutable_camera() = od.camera.to_proto(); + *proto.mutable_vision_service() = v2::to_proto(od.vision_service); + *proto.mutable_camera() = v2::to_proto(od.camera); return proto; } @@ -127,7 +127,7 @@ std::vector from_proto( Motion::plan_status_with_id from_proto(const service::motion::v1::PlanStatusWithID& proto) { Motion::plan_status_with_id pswi; pswi.execution_id = proto.execution_id(); - pswi.component_name = Name::from_proto(proto.component_name()); + pswi.component_name = v2::from_proto(proto.component_name()); pswi.plan_id = proto.plan_id(); pswi.status = from_proto(proto.status()); @@ -153,7 +153,7 @@ Motion::plan plan_from_proto(const service::motion::v1::Plan& proto) { Motion::plan plan; plan.id = proto.id(); plan.execution_id = proto.execution_id(); - plan.component_name = Name::from_proto(proto.component_name()); + plan.component_name = v2::from_proto(proto.component_name()); plan.steps = steps_from_proto(proto.steps()); return plan; } @@ -189,7 +189,7 @@ bool MotionClient::move(const pose_in_frame& destination, return make_client_helper(this, *stub_, &StubType::Move) .with(extra, [&](auto& request) { - *request.mutable_component_name() = component_name.to_proto(); + *request.mutable_component_name() = v2::to_proto(component_name); *request.mutable_destination() = v2::to_proto(destination); if (constraints) { *request.mutable_constraints() = to_proto(*constraints); @@ -212,8 +212,8 @@ std::string MotionClient::move_on_map( .with(extra, [&](auto& request) { *request.mutable_destination() = v2::to_proto(destination); - *request.mutable_component_name() = component_name.to_proto(); - *request.mutable_slam_service_name() = slam_name.to_proto(); + *request.mutable_component_name() = v2::to_proto(component_name); + *request.mutable_slam_service_name() = v2::to_proto(slam_name); for (const auto& obstacle : obstacles) { *request.mutable_obstacles()->Add() = v2::to_proto(obstacle); @@ -239,8 +239,8 @@ std::string MotionClient::move_on_globe( .with(extra, [&](auto& request) { *request.mutable_destination() = v2::to_proto(destination); - *request.mutable_component_name() = component_name.to_proto(); - *request.mutable_movement_sensor_name() = movement_sensor_name.to_proto(); + *request.mutable_component_name() = v2::to_proto(component_name); + *request.mutable_movement_sensor_name() = v2::to_proto(movement_sensor_name); if (heading && !isnan(*heading)) { request.set_heading(*heading); @@ -265,7 +265,7 @@ pose_in_frame MotionClient::get_pose( return make_client_helper(this, *stub_, &StubType::GetPose) .with(extra, [&](auto& request) { - *request.mutable_component_name() = component_name.to_proto(); + *request.mutable_component_name() = v2::to_proto(component_name); *request.mutable_destination_frame() = destination_frame; *request.mutable_supplemental_transforms() = impl::to_repeated_field(supplemental_transforms); @@ -275,7 +275,7 @@ pose_in_frame MotionClient::get_pose( void MotionClient::stop_plan(const Name& name, const ProtoStruct& extra) { return make_client_helper(this, *stub_, &StubType::StopPlan) - .with(extra, [&](auto& request) { *request.mutable_component_name() = name.to_proto(); }) + .with(extra, [&](auto& request) { *request.mutable_component_name() = v2::to_proto(name); }) .invoke(); } @@ -287,7 +287,7 @@ std::pair> Motio return make_client_helper(this, *stub_, &StubType::GetPlan) .with(extra, [&](auto& request) { - *request.mutable_component_name() = component_name.to_proto(); + *request.mutable_component_name() = v2::to_proto(component_name); request.set_last_plan_only(last_plan_only); if (execution_id) { *request.mutable_execution_id() = *execution_id; diff --git a/src/viam/sdk/services/private/motion_server.cpp b/src/viam/sdk/services/private/motion_server.cpp index 1dad3ef57..eee9360cf 100644 --- a/src/viam/sdk/services/private/motion_server.cpp +++ b/src/viam/sdk/services/private/motion_server.cpp @@ -62,7 +62,7 @@ service::motion::v1::PlanStep to_proto(const Motion::steps::step& step) { service::motion::v1::Plan to_proto(const Motion::plan& plan) { service::motion::v1::Plan proto; *proto.mutable_id() = plan.id; - *proto.mutable_component_name() = plan.component_name.to_proto(); + *proto.mutable_component_name() = v2::to_proto(plan.component_name); *proto.mutable_execution_id() = plan.execution_id; for (const auto& step : plan.steps.steps) { *proto.mutable_steps()->Add() = to_proto(step); @@ -86,7 +86,7 @@ service::motion::v1::PlanStatusWithID to_proto(const Motion::plan_status_with_id service::motion::v1::PlanStatusWithID proto; *proto.mutable_execution_id() = pswi.execution_id; - *proto.mutable_component_name() = pswi.component_name.to_proto(); + *proto.mutable_component_name() = v2::to_proto(pswi.component_name); *proto.mutable_plan_id() = pswi.plan_id; *proto.mutable_status() = to_proto(pswi.status); @@ -95,8 +95,8 @@ service::motion::v1::PlanStatusWithID to_proto(const Motion::plan_status_with_id obstacle_detector from_proto(const service::motion::v1::ObstacleDetector& proto) { obstacle_detector oc; - oc.vision_service = Name::from_proto(proto.vision_service()); - oc.camera = Name::from_proto(proto.camera()); + oc.vision_service = v2::from_proto(proto.vision_service()); + oc.camera = v2::from_proto(proto.camera()); return oc; } @@ -187,7 +187,7 @@ ::grpc::Status MotionServer::Move(::grpc::ServerContext*, } const bool success = motion->move(v2::from_proto(request->destination()), - Name::from_proto(request->component_name()), + v2::from_proto(request->component_name()), std::move(ws), std::move(constraints), helper.getExtra()); @@ -202,8 +202,8 @@ ::grpc::Status MotionServer::MoveOnMap( return make_service_helper( "MotionServer::MoveOnMap", this, request)([&](auto& helper, auto& motion) { const auto destination = v2::from_proto(request->destination()); - const auto component_name = Name::from_proto(request->component_name()); - const auto slam_name = Name::from_proto(request->slam_service_name()); + const auto component_name = v2::from_proto(request->component_name()); + const auto slam_name = v2::from_proto(request->slam_service_name()); std::shared_ptr mc; if (request->has_motion_configuration()) { @@ -230,8 +230,8 @@ ::grpc::Status MotionServer::MoveOnGlobe( return make_service_helper( "MotionServer::MoveOnGlobe", this, request)([&](auto& helper, auto& motion) { const auto destination = v2::from_proto(request->destination()); - const auto component_name = Name::from_proto(request->component_name()); - const auto movement_sensor_name = Name::from_proto(request->movement_sensor_name()); + const auto component_name = v2::from_proto(request->component_name()); + const auto movement_sensor_name = v2::from_proto(request->movement_sensor_name()); const std::vector obstacles = impl::from_repeated_field(request->obstacles()); const std::vector bounding_regions = impl::from_repeated_field(request->bounding_regions()); @@ -266,7 +266,7 @@ ::grpc::Status MotionServer::GetPose( ::viam::service::motion::v1::GetPoseResponse* response) noexcept { return make_service_helper( "MotionServer::GetPose", this, request)([&](auto& helper, auto& motion) { - const auto& component_name = Name::from_proto(request->component_name()); + const auto& component_name = v2::from_proto(request->component_name()); const std::string& destination_frame = request->destination_frame(); std::vector supplemental_transforms; for (const auto& proto_transform : request->supplemental_transforms()) { @@ -284,7 +284,7 @@ ::grpc::Status MotionServer::GetPlan( ::viam::service::motion::v1::GetPlanResponse* response) noexcept { return make_service_helper( "MotionServer::GetPlan", this, request)([&](auto& helper, auto& motion) { - const auto& component_name = Name::from_proto(request->component_name()); + const auto& component_name = v2::from_proto(request->component_name()); Motion::plan_with_status plan; std::vector replan_history; const bool last_plan_only(request->last_plan_only()); @@ -337,7 +337,7 @@ ::grpc::Status MotionServer::StopPlan(::grpc::ServerContext*, ::viam::service::motion::v1::StopPlanResponse*) noexcept { return make_service_helper( "MotionServer::StopPlan", this, request)([&](auto& helper, auto& motion) { - const auto& component_name = Name::from_proto(request->component_name()); + const auto& component_name = v2::from_proto(request->component_name()); motion->stop_plan(component_name, helper.getExtra()); }); diff --git a/src/viam/sdk/tests/test_resource.cpp b/src/viam/sdk/tests/test_resource.cpp index db950355a..5bc5fe383 100644 --- a/src/viam/sdk/tests/test_resource.cpp +++ b/src/viam/sdk/tests/test_resource.cpp @@ -52,7 +52,7 @@ BOOST_AUTO_TEST_CASE(test_name) { BOOST_CHECK_EQUAL(name1.name(), "name"); BOOST_CHECK_EQUAL(name1.short_name(), "remote:name"); BOOST_CHECK_EQUAL(name1.to_string(), "ns:service:st/remote:name"); - BOOST_CHECK_EQUAL(Name::from_proto(name1.to_proto()), name1); + BOOST_CHECK_EQUAL(v2::from_proto(v2::to_proto(name1)), name1); Name name2(API::from_string("ns:service:st"), "remote1:remote2", "name"); BOOST_CHECK_EQUAL(name2.api().to_string(), "ns:service:st"); @@ -60,7 +60,7 @@ BOOST_AUTO_TEST_CASE(test_name) { BOOST_CHECK_EQUAL(name2.name(), "name"); BOOST_CHECK_EQUAL(name2.short_name(), "remote1:remote2:name"); BOOST_CHECK_EQUAL(name2.to_string(), "ns:service:st/remote1:remote2:name"); - BOOST_CHECK_EQUAL(Name::from_proto(name2.to_proto()), name2); + BOOST_CHECK_EQUAL(v2::from_proto(v2::to_proto(name2)), name2); Name name3 = Name::from_string("ns:component:st/name"); BOOST_CHECK_EQUAL(name3.api().to_string(), "ns:component:st"); @@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(test_name) { BOOST_CHECK_EQUAL(name3.name(), "name"); BOOST_CHECK_EQUAL(name3.short_name(), "name"); BOOST_CHECK_EQUAL(name3.to_string(), "ns:component:st/name"); - BOOST_CHECK_EQUAL(Name::from_proto(name3.to_proto()), name3); + BOOST_CHECK_EQUAL(v2::from_proto(v2::to_proto(name3)), name3); BOOST_CHECK_THROW(Name::from_string("ns:service:#st/remote:name"), Exception); } From 75d6d3a7bbbefb2a74ca7513bd42a4b47984ad23 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Fri, 13 Dec 2024 10:53:49 -0500 Subject: [PATCH 05/20] class to struct --- src/viam/sdk/config/resource.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/viam/sdk/config/resource.hpp b/src/viam/sdk/config/resource.hpp index b1a703a6a..95aadeae5 100644 --- a/src/viam/sdk/config/resource.hpp +++ b/src/viam/sdk/config/resource.hpp @@ -13,8 +13,7 @@ namespace viam { namespace sdk { -class ResourceLevelServiceConfig { - public: +struct ResourceLevelServiceConfig { std::string type; ProtoStruct attributes; ProtoValue converted_attributes; From 1d26302d220ace64c57a7e2e837cd9e9ecdeeb80 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Fri, 13 Dec 2024 14:12:58 -0500 Subject: [PATCH 06/20] insulate resource and utils --- src/viam/sdk/common/client_helper.hpp | 20 ++++-- src/viam/sdk/common/utils.cpp | 9 ++- src/viam/sdk/common/utils.hpp | 2 + src/viam/sdk/config/resource.cpp | 98 ++++++++++++++++----------- src/viam/sdk/config/resource.hpp | 38 +++++++++-- src/viam/sdk/module/service.cpp | 6 +- src/viam/sdk/tests/test_common.cpp | 2 + src/viam/sdk/tests/test_resource.cpp | 8 ++- 8 files changed, 126 insertions(+), 57 deletions(-) diff --git a/src/viam/sdk/common/client_helper.hpp b/src/viam/sdk/common/client_helper.hpp index 95d1f8f09..ca77c9fa9 100644 --- a/src/viam/sdk/common/client_helper.hpp +++ b/src/viam/sdk/common/client_helper.hpp @@ -1,18 +1,27 @@ #pragma once -#include -#include - #include #include #include #include +namespace grpc { + +class Status; + +class ClientContext; + +template +class ClientReaderInterface; + +} // namespace grpc + namespace viam { namespace sdk { namespace client_helper_details { [[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status&) noexcept; + } // namespace client_helper_details // Method type for a gRPC call that returns a response message type. @@ -59,7 +68,8 @@ class ClientHelper { ProtoValue value = key->second; debug_key_ = *value.get(); } - *request_.mutable_extra() = v2::to_proto(extra); + + proto_convert_details::to_proto{}(extra, request_.mutable_extra()); return with(std::forward(rsc)); } @@ -102,7 +112,7 @@ class ClientHelper { while (reader->Read(&response_)) { if (!rhc(response_)) { cancelled_by_handler = true; - static_cast<::grpc::ClientContext*>(ctx)->TryCancel(); + ctx.try_cancel(); break; } } diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index b4c2d8968..bcc2f432e 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -4,13 +4,16 @@ #include #include +#include +#include +#include + #include #include #include #include #include #include -#include #include @@ -164,6 +167,10 @@ 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); } diff --git a/src/viam/sdk/common/utils.hpp b/src/viam/sdk/common/utils.hpp index 795521e48..e2dcf9c64 100644 --- a/src/viam/sdk/common/utils.hpp +++ b/src/viam/sdk/common/utils.hpp @@ -97,6 +97,8 @@ class ClientContext { ClientContext(); ~ClientContext(); + void try_cancel(); + operator grpc::ClientContext*(); operator const grpc::ClientContext*() const; diff --git a/src/viam/sdk/config/resource.cpp b/src/viam/sdk/config/resource.cpp index ed772a328..3a2b98938 100644 --- a/src/viam/sdk/config/resource.cpp +++ b/src/viam/sdk/config/resource.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include #include @@ -18,6 +19,26 @@ namespace viam { namespace sdk { +ResourceConfig::ResourceConfig(std::string type, + std::string name, + std::string namespace_, + ProtoStruct attributes, + std::string api, + Model model, + LinkConfig frame) + : api_({kRDK, type, ""}), + frame_(std::move(frame)), + model_(std::move(model)), + name_(std::move(name)), + namespace__(std::move(namespace_)), + type_(std::move(type)), + attributes_(std::move(attributes)) { + if (api.find(':') != std::string::npos) { + api_ = API::from_string(std::move(api)); + } + fix_api(); +} + ResourceConfig::ResourceConfig(std::string type) : api_({kRDK, type, ""}), type_(std::move(type)) {} Name ResourceConfig::resource_name() { @@ -59,6 +80,14 @@ const std::string& ResourceConfig::type() const { return type_; } +const std::vector& viam::sdk::ResourceConfig::depends_on() const { + return depends_on_; +} + +const std::vector& viam::sdk::ResourceConfig::service_config() const { + return service_config_; +} + const ProtoStruct& ResourceConfig::attributes() const { return attributes_; } @@ -91,54 +120,41 @@ void ResourceConfig::fix_api() { } } -ResourceConfig ResourceConfig::from_proto(const viam::app::v1::ComponentConfig& proto_cfg) { - ResourceConfig resource(proto_cfg.type()); - resource.name_ = proto_cfg.name(); - resource.namespace__ = proto_cfg.namespace_(); - resource.type_ = proto_cfg.type(); - resource.attributes_ = v2::from_proto(proto_cfg.attributes()); - const std::string& api = proto_cfg.api(); - if (api.find(':') != std::string::npos) { - resource.api_ = API::from_string(api); - } - resource.model_ = Model::from_str(proto_cfg.model()); +namespace proto_convert_details { - resource.fix_api(); - - if (proto_cfg.has_frame()) { - resource.frame_ = v2::from_proto(proto_cfg.frame()); - } +void to_proto::operator()( + const ResourceLevelServiceConfig& self, app::v1::ResourceLevelServiceConfig* proto) const { + *proto->mutable_type() = self.type; + *proto->mutable_attributes() = v2::to_proto(self.attributes); +} - return resource; -}; +void to_proto::operator()(const ResourceConfig& self, + app::v1::ComponentConfig* proto) const { + *proto->mutable_service_configs() = impl::to_repeated_field(self.service_config()); -viam::app::v1::ComponentConfig ResourceConfig::to_proto() const { - viam::app::v1::ComponentConfig proto_cfg; - const google::protobuf::Struct s = v2::to_proto(attributes_); - const google::protobuf::RepeatedPtrField - service_configs; + *proto->mutable_name() = self.name(); + *proto->mutable_namespace_() = self.namespace_(); + *proto->mutable_type() = self.type(); + *proto->mutable_api() = self.api().to_string(); + *proto->mutable_model() = self.model().to_string(); + *proto->mutable_attributes() = v2::to_proto(self.attributes()); - for (const auto& svc_cfg : service_config_) { - viam::app::v1::ResourceLevelServiceConfig cfg; - *cfg.mutable_type() = svc_cfg.type; - *cfg.mutable_attributes() = v2::to_proto(svc_cfg.attributes); - *proto_cfg.mutable_service_configs()->Add() = cfg; - } + proto->mutable_depends_on()->Assign(self.depends_on().begin(), self.depends_on().end()); - *proto_cfg.mutable_name() = name_; - *proto_cfg.mutable_namespace_() = namespace__; - *proto_cfg.mutable_type() = type_; - *proto_cfg.mutable_api() = api_.to_string(); - const std::string mm = model_.to_string(); - *proto_cfg.mutable_model() = mm; - *proto_cfg.mutable_attributes() = v2::to_proto(attributes_); - for (const auto& dep : depends_on_) { - *proto_cfg.mutable_depends_on()->Add() = dep; - } - *proto_cfg.mutable_frame() = v2::to_proto(frame_); + *proto->mutable_frame() = v2::to_proto(self.frame()); +} - return proto_cfg; +ResourceConfig from_proto::operator()( + const app::v1::ComponentConfig* proto) const { + return ResourceConfig(proto->type(), + proto->name(), + proto->namespace_(), + v2::from_proto(proto->attributes()), + proto->api(), + Model::from_str(proto->model()), + proto->has_frame() ? v2::from_proto(proto->frame()) : LinkConfig{}); } +} // namespace proto_convert_details } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/config/resource.hpp b/src/viam/sdk/config/resource.hpp index 95aadeae5..4fcaf1725 100644 --- a/src/viam/sdk/config/resource.hpp +++ b/src/viam/sdk/config/resource.hpp @@ -3,13 +3,22 @@ #include #include -#include -#include - +#include #include #include #include +namespace viam { +namespace app { +namespace v1 { + +class ComponentConfig; +class ResourceLevelServiceConfig; + +} // namespace v1 +} // namespace app +} // namespace viam + namespace viam { namespace sdk { @@ -21,8 +30,6 @@ struct ResourceLevelServiceConfig { class ResourceConfig { public: - static ResourceConfig from_proto(const viam::app::v1::ComponentConfig& proto_cfg); - viam::app::v1::ComponentConfig to_proto() const; ResourceConfig(std::string type, std::string name, std::string namespace_, @@ -40,6 +47,8 @@ class ResourceConfig { const std::string& name() const; const std::string& namespace_() const; const std::string& type() const; + const std::vector& depends_on() const; + const std::vector& service_config() const; const ProtoStruct& attributes() const; private: @@ -57,5 +66,24 @@ class ResourceConfig { void fix_api(); }; +namespace proto_convert_details { + +template <> +struct to_proto { + void operator()(const ResourceLevelServiceConfig&, app::v1::ResourceLevelServiceConfig*) const; +}; + +template <> +struct to_proto { + void operator()(const ResourceConfig&, app::v1::ComponentConfig*) const; +}; + +template <> +struct from_proto { + ResourceConfig operator()(const app::v1::ComponentConfig*) const; +}; + +} // namespace proto_convert_details + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index b1bad21ac..430fbf2b7 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -73,7 +73,7 @@ ::grpc::Status ModuleService::AddResource(::grpc::ServerContext*, const ::viam::module::v1::AddResourceRequest* request, ::viam::module::v1::AddResourceResponse*) { const viam::app::v1::ComponentConfig& proto = request->config(); - const ResourceConfig cfg = ResourceConfig::from_proto(proto); + const ResourceConfig cfg = v2::from_proto(proto); const std::lock_guard lock(lock_); std::shared_ptr res; @@ -101,7 +101,7 @@ ::grpc::Status ModuleService::ReconfigureResource( const ::viam::module::v1::ReconfigureResourceRequest* request, ::viam::module::v1::ReconfigureResourceResponse*) { const viam::app::v1::ComponentConfig& proto = request->config(); - ResourceConfig cfg = ResourceConfig::from_proto(proto); + ResourceConfig cfg = v2::from_proto(proto); const Dependencies deps = get_dependencies_(request->dependencies(), cfg.name()); @@ -150,7 +150,7 @@ ::grpc::Status ModuleService::ValidateConfig( const ::viam::module::v1::ValidateConfigRequest* request, ::viam::module::v1::ValidateConfigResponse* response) { const viam::app::v1::ComponentConfig& proto = request->config(); - ResourceConfig cfg = ResourceConfig::from_proto(proto); + ResourceConfig cfg = v2::from_proto(proto); const std::shared_ptr reg = Registry::lookup_model(cfg.api(), cfg.model()); diff --git a/src/viam/sdk/tests/test_common.cpp b/src/viam/sdk/tests/test_common.cpp index b804bf0af..ca4b87398 100644 --- a/src/viam/sdk/tests/test_common.cpp +++ b/src/viam/sdk/tests/test_common.cpp @@ -2,6 +2,8 @@ #include +#include + #include #include diff --git a/src/viam/sdk/tests/test_resource.cpp b/src/viam/sdk/tests/test_resource.cpp index 5bc5fe383..cba417847 100644 --- a/src/viam/sdk/tests/test_resource.cpp +++ b/src/viam/sdk/tests/test_resource.cpp @@ -2,6 +2,10 @@ #include #include + +#include +#include + #include #include #include @@ -235,7 +239,7 @@ BOOST_AUTO_TEST_CASE(test_resource) { *frame.mutable_translation() = t; *proto_cfg.mutable_frame() = frame; - ResourceConfig resource2 = ResourceConfig::from_proto(proto_cfg); + ResourceConfig resource2 = v2::from_proto(proto_cfg); BOOST_CHECK_EQUAL(resource2.name(), "name"); BOOST_CHECK_EQUAL(resource2.namespace_(), "ns"); BOOST_CHECK_EQUAL(resource2.type(), "type"); @@ -252,7 +256,7 @@ BOOST_AUTO_TEST_CASE(test_resource) { BOOST_CHECK_EQUAL(value.number_value(), 1); *proto_cfg.mutable_api() = "ns:component:test"; - BOOST_CHECK_THROW(ResourceConfig::from_proto(proto_cfg), Exception); + BOOST_CHECK_THROW(v2::from_proto(proto_cfg), Exception); } } // namespace sdktests From 2fa362f0b62aaa52e54227148718f68ba84a422a Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Fri, 13 Dec 2024 14:19:03 -0500 Subject: [PATCH 07/20] symmetric ws --- src/viam/sdk/common/client_helper.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/viam/sdk/common/client_helper.hpp b/src/viam/sdk/common/client_helper.hpp index ca77c9fa9..f5d816bc9 100644 --- a/src/viam/sdk/common/client_helper.hpp +++ b/src/viam/sdk/common/client_helper.hpp @@ -20,6 +20,7 @@ namespace viam { namespace sdk { namespace client_helper_details { + [[noreturn]] void errorHandlerReturnedUnexpectedly(const ::grpc::Status&) noexcept; } // namespace client_helper_details From 924bd864b648f3b714178553ee08480aadab6957 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Fri, 13 Dec 2024 14:19:15 -0500 Subject: [PATCH 08/20] remove spurious ret --- src/viam/sdk/common/utils.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index bcc2f432e..b914ab0e2 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -73,7 +73,6 @@ std::chrono::microseconds from_proto::operator()( from_nanos += sc::microseconds(1); } return from_seconds + from_nanos; - return std::chrono::microseconds(); } void to_proto::operator()(const response_metadata& self, From ecf734b655ec3a1939c2949df3e0c65ae19251b9 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Fri, 13 Dec 2024 14:25:01 -0500 Subject: [PATCH 09/20] formatting and defaulted ctors --- src/viam/sdk/resource/resource_api.hpp | 14 +++++++++----- src/viam/sdk/resource/resource_manager.hpp | 4 ++-- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/viam/sdk/resource/resource_api.hpp b/src/viam/sdk/resource/resource_api.hpp index 868d14a37..9a87cf6b4 100644 --- a/src/viam/sdk/resource/resource_api.hpp +++ b/src/viam/sdk/resource/resource_api.hpp @@ -23,8 +23,9 @@ namespace sdk { /// @brief Defines a resource's namespace (e.g., `RDK`) and type (e.g., component or service). class APIType { public: + APIType() = default; APIType(std::string namespace_, std::string resource_type); - APIType() {}; + virtual std::string to_string() const; const std::string& type_namespace() const; @@ -40,13 +41,16 @@ class APIType { /// @class API /// @brief Extends `APIType` to additionally define a resource's subtype (e.g., `camera`). +// TODO: Maybe just merge these two classes or at least use composition rather than inheritance class API : public APIType { public: - virtual std::string to_string() const override; - API() {}; + static API from_string(std::string api); + + API() = default; API(std::string namespace_, std::string resource_type, std::string resource_subtype); API(APIType type, std::string resource_subtype); - static API from_string(std::string api); + + virtual std::string to_string() const override; const std::string& resource_subtype() const; void set_resource_subtype(const std::string& subtype); @@ -75,7 +79,7 @@ class Name { static Name from_string(std::string name); Name(API api, std::string remote_name, std::string name); - Name() {}; + Name() = default; std::string short_name() const; std::string to_string() const; diff --git a/src/viam/sdk/resource/resource_manager.hpp b/src/viam/sdk/resource/resource_manager.hpp index 0a8f62c32..b4dbb0e5d 100644 --- a/src/viam/sdk/resource/resource_manager.hpp +++ b/src/viam/sdk/resource/resource_manager.hpp @@ -20,6 +20,8 @@ namespace sdk { /// @brief Defines a resource manager for use by anything that tracks resources. class ResourceManager { public: + ResourceManager() = default; + /// @brief Returns a resource. /// @param name the name of the desired resource. /// @throws `Exception` if the desired resource does not exist. @@ -60,8 +62,6 @@ class ResourceManager { /// @brief Returns a reference to the existing resources within the manager. const std::unordered_map>& resources() const; - ResourceManager() {}; - private: std::mutex lock_; std::unordered_map> resources_; From c768df8125503663ed26d9fbc6e47cbdc87a853a Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Mon, 16 Dec 2024 13:55:20 -0500 Subject: [PATCH 10/20] de-insulate grpc client context --- src/viam/sdk/common/client_helper.hpp | 2 -- src/viam/sdk/common/utils.hpp | 7 +------ 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/viam/sdk/common/client_helper.hpp b/src/viam/sdk/common/client_helper.hpp index f5d816bc9..926b03ab6 100644 --- a/src/viam/sdk/common/client_helper.hpp +++ b/src/viam/sdk/common/client_helper.hpp @@ -9,8 +9,6 @@ namespace grpc { class Status; -class ClientContext; - template class ClientReaderInterface; diff --git a/src/viam/sdk/common/utils.hpp b/src/viam/sdk/common/utils.hpp index e2dcf9c64..cb3c76532 100644 --- a/src/viam/sdk/common/utils.hpp +++ b/src/viam/sdk/common/utils.hpp @@ -3,6 +3,7 @@ #include #include +#include #include #include @@ -17,12 +18,6 @@ class Timestamp; } // namespace protobuf } // namespace google -namespace grpc { - -class ClientContext; - -} - namespace viam { namespace common { namespace v1 { From b18f477fd3039d969ceb2ea6f495e10c47aaab09 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 17 Dec 2024 11:20:02 -0500 Subject: [PATCH 11/20] put grpc includes back --- src/viam/sdk/common/client_helper.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/viam/sdk/common/client_helper.hpp b/src/viam/sdk/common/client_helper.hpp index 926b03ab6..c68e7fe85 100644 --- a/src/viam/sdk/common/client_helper.hpp +++ b/src/viam/sdk/common/client_helper.hpp @@ -1,5 +1,8 @@ #pragma once +#include +#include + #include #include #include @@ -9,9 +12,6 @@ namespace grpc { class Status; -template -class ClientReaderInterface; - } // namespace grpc namespace viam { From a20fb42cd175d38fac44bb76b2f264d53d03f657 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 17 Dec 2024 14:12:48 -0500 Subject: [PATCH 12/20] generate header for grpc client fwd --- CMakeLists.txt | 4 ++++ src/viam/sdk/CMakeLists.txt | 5 ++++- src/viam/sdk/common/client_helper.hpp | 4 +--- src/viam/sdk/common/utils.hpp | 2 +- 4 files changed, 10 insertions(+), 5 deletions(-) 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..d969f4528 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}/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.hpp b/src/viam/sdk/common/client_helper.hpp index c68e7fe85..bea8332d2 100644 --- a/src/viam/sdk/common/client_helper.hpp +++ b/src/viam/sdk/common/client_helper.hpp @@ -1,9 +1,7 @@ #pragma once -#include -#include - #include +#include #include #include #include diff --git a/src/viam/sdk/common/utils.hpp b/src/viam/sdk/common/utils.hpp index cb3c76532..24e7af212 100644 --- a/src/viam/sdk/common/utils.hpp +++ b/src/viam/sdk/common/utils.hpp @@ -3,8 +3,8 @@ #include #include -#include +#include #include #include #include From c223260251c21677306927404cd85976ff2aa3d9 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 17 Dec 2024 14:16:02 -0500 Subject: [PATCH 13/20] use add --- src/viam/sdk/config/resource.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/config/resource.cpp b/src/viam/sdk/config/resource.cpp index 3a2b98938..203a23ecb 100644 --- a/src/viam/sdk/config/resource.cpp +++ b/src/viam/sdk/config/resource.cpp @@ -139,7 +139,7 @@ void to_proto::operator()(const ResourceConfig& self, *proto->mutable_model() = self.model().to_string(); *proto->mutable_attributes() = v2::to_proto(self.attributes()); - proto->mutable_depends_on()->Assign(self.depends_on().begin(), self.depends_on().end()); + proto->mutable_depends_on()->Add(self.depends_on().begin(), self.depends_on().end()); *proto->mutable_frame() = v2::to_proto(self.frame()); } From 6978fdf61812079f9103a43c46bf23fd8923f81e Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 17 Dec 2024 14:43:05 -0500 Subject: [PATCH 14/20] use explicit ctor/assign --- src/viam/sdk/config/resource.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/viam/sdk/config/resource.cpp b/src/viam/sdk/config/resource.cpp index 203a23ecb..45402166a 100644 --- a/src/viam/sdk/config/resource.cpp +++ b/src/viam/sdk/config/resource.cpp @@ -139,7 +139,8 @@ void to_proto::operator()(const ResourceConfig& self, *proto->mutable_model() = self.model().to_string(); *proto->mutable_attributes() = v2::to_proto(self.attributes()); - proto->mutable_depends_on()->Add(self.depends_on().begin(), self.depends_on().end()); + *proto->mutable_depends_on() = ::google::protobuf::RepeatedPtrField( + self.depends_on().begin(), self.depends_on().end()); *proto->mutable_frame() = v2::to_proto(self.frame()); } From f59997f597572379240a6af29c9dfe138c38920d Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:28:42 -0500 Subject: [PATCH 15/20] initialize unique_ptr --- src/viam/sdk/common/utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index b914ab0e2..fdd03b6da 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -151,7 +151,7 @@ ProtoStruct with_debug_entry(ProtoStruct&& map) { return map; } -ClientContext::ClientContext() { +ClientContext::ClientContext() : wrapped_context_(std::make_unique()) { set_client_ctx_authority_(); add_viam_client_version_(); } From 772efcbab405c65c7618b4520245521f246c1fab Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:53:06 -0500 Subject: [PATCH 16/20] client context out of utils --- src/viam/sdk/common/client_helper.cpp | 36 +++++++++++++++++++ src/viam/sdk/common/client_helper.hpp | 24 ++++++++++++- 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 +- 6 files changed, 61 insertions(+), 58 deletions(-) diff --git a/src/viam/sdk/common/client_helper.cpp b/src/viam/sdk/common/client_helper.cpp index 84990153d..8efa5d709 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 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); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/common/client_helper.hpp b/src/viam/sdk/common/client_helper.hpp index bea8332d2..2d9aa2183 100644 --- a/src/viam/sdk/common/client_helper.hpp +++ b/src/viam/sdk/common/client_helper.hpp @@ -4,7 +4,6 @@ #include #include #include -#include namespace grpc { @@ -21,6 +20,29 @@ 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 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_; +}; + // Method type for a gRPC call that returns a response message type. template using SyncMethodType = ::grpc::Status (StubType::*)(::grpc::ClientContext*, 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 24e7af212..f47442750 100644 --- a/src/viam/sdk/common/utils.hpp +++ b/src/viam/sdk/common/utils.hpp @@ -4,7 +4,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 From fc38fff843dda1c4b831f86fd8342df5e3ca976b Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 18 Dec 2024 07:50:48 -0500 Subject: [PATCH 17/20] commit configure input file --- src/viam/sdk/common/grpc_client_fwd.hpp.in | 55 ++++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 src/viam/sdk/common/grpc_client_fwd.hpp.in diff --git a/src/viam/sdk/common/grpc_client_fwd.hpp.in b/src/viam/sdk/common/grpc_client_fwd.hpp.in new file mode 100644 index 000000000..e6ba5799f --- /dev/null +++ b/src/viam/sdk/common/grpc_client_fwd.hpp.in @@ -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. +// 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 From 0262a08e6fcf32f514dc68921f45e6a5ec6136e8 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 18 Dec 2024 10:21:30 -0500 Subject: [PATCH 18/20] use the typedef --- src/viam/sdk/common/client_helper.cpp | 6 +++--- src/viam/sdk/common/client_helper.hpp | 16 +++++----------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/src/viam/sdk/common/client_helper.cpp b/src/viam/sdk/common/client_helper.cpp index 8efa5d709..1d98249b6 100644 --- a/src/viam/sdk/common/client_helper.cpp +++ b/src/viam/sdk/common/client_helper.cpp @@ -21,18 +21,18 @@ namespace client_helper_details { } // namespace client_helper_details -ClientContext::ClientContext() : wrapped_context_(std::make_unique()) { +ClientContext::ClientContext() : wrapped_context_(std::make_unique()) { set_client_ctx_authority_(); add_viam_client_version_(); } ClientContext::~ClientContext() = default; -ClientContext::operator const grpc::ClientContext*() const { +ClientContext::operator const GrpcClientContext*() const { return wrapped_context_.get(); } -ClientContext::operator grpc::ClientContext*() { +ClientContext::operator GrpcClientContext*() { return wrapped_context_.get(); } diff --git a/src/viam/sdk/common/client_helper.hpp b/src/viam/sdk/common/client_helper.hpp index 734d1bc28..578ae694c 100644 --- a/src/viam/sdk/common/client_helper.hpp +++ b/src/viam/sdk/common/client_helper.hpp @@ -11,12 +11,6 @@ class Status; } // namespace grpc -namespace grpc { - -class Status; - -} // namespace grpc - namespace viam { namespace sdk { @@ -38,27 +32,27 @@ class ClientContext { void try_cancel(); - operator grpc::ClientContext*(); - operator const grpc::ClientContext*() const; + 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_; + 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&); + StubType::*)(GrpcClientContext*, const RequestType&); template Date: Wed, 18 Dec 2024 10:23:52 -0500 Subject: [PATCH 19/20] use client reader fwd --- src/viam/sdk/common/client_helper.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/viam/sdk/common/client_helper.hpp b/src/viam/sdk/common/client_helper.hpp index 578ae694c..27171c9e0 100644 --- a/src/viam/sdk/common/client_helper.hpp +++ b/src/viam/sdk/common/client_helper.hpp @@ -51,8 +51,8 @@ using SyncMethodType = ::grpc::Status (StubType::*)(GrpcClientContext*, // Method type for a gRPC call that returns a stream of response message type. template -using StreamingMethodType = std::unique_ptr<::grpc::ClientReaderInterface> ( - StubType::*)(GrpcClientContext*, const RequestType&); +using StreamingMethodType = std::unique_ptr> (StubType::*)( + GrpcClientContext*, const RequestType&); template Date: Mon, 23 Dec 2024 12:23:11 -0500 Subject: [PATCH 20/20] fix installation of generated file --- src/viam/sdk/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/viam/sdk/CMakeLists.txt b/src/viam/sdk/CMakeLists.txt index d969f4528..fe5982e1f 100644 --- a/src/viam/sdk/CMakeLists.txt +++ b/src/viam/sdk/CMakeLists.txt @@ -139,7 +139,7 @@ target_sources(viamsdk PUBLIC FILE_SET viamsdk_public_includes TYPE HEADERS BASE_DIRS ../.. - ${CMAKE_CURRENT_BINARY_DIR} + ${CMAKE_CURRENT_BINARY_DIR}/../.. FILES ../../viam/sdk/common/client_helper.hpp ../../viam/sdk/common/exception.hpp @@ -192,7 +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 + ${CMAKE_CURRENT_BINARY_DIR}/../../viam/sdk/common/grpc_client_fwd.hpp ) set_target_properties(