RSDK-9449 Miscellaneous further proto cleanups (#339)

Author: lia-viam

src/viam/sdk/common/client_helper.hpp

@@ -8,11 +8,19 @@
 #include <viam/sdk/common/proto_value.hpp>
 #include <viam/sdk/common/utils.hpp>
 
+namespace grpc {
+
+class Status;
+
+}  // 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 +67,8 @@ class ClientHelper {
             ProtoValue value = key->second;
             debug_key_ = *value.get<std::string>();
         }
-        *request_.mutable_extra() = v2::to_proto(extra);
+
+        proto_convert_details::to_proto<ProtoStruct>{}(extra, request_.mutable_extra());
         return with(std::forward<RequestSetupCallable>(rsc));
     }
 
@@ -102,7 +111,7 @@ class ClientHelper {
         while (reader->Read(&response_)) {
             if (!rhc(response_)) {
                 cancelled_by_handler = true;
-                static_cast<::grpc::ClientContext*>(ctx)->TryCancel();
+                ctx.try_cancel();
                 break;
             }
         }

src/viam/sdk/common/service_helper.hpp

@@ -2,6 +2,8 @@
 
 #include <type_traits>
 
+#include <grpcpp/support/status.h>
+
 #include <viam/sdk/resource/resource_server_base.hpp>
 
 namespace viam {

src/viam/sdk/common/utils.cpp

@@ -4,13 +4,16 @@
 #include <unordered_map>
 #include <vector>
 
+#include <google/protobuf/duration.pb.h>
+#include <google/protobuf/timestamp.pb.h>
+#include <grpcpp/client_context.h>
+
 #include <boost/algorithm/string.hpp>
 #include <boost/blank.hpp>
 #include <boost/log/core.hpp>
 #include <boost/log/expressions.hpp>
 #include <boost/log/trivial.hpp>
 #include <boost/optional/optional.hpp>
-#include <grpcpp/client_context.h>
 
 #include <viam/api/common/v1/common.pb.h>
 
@@ -22,52 +25,44 @@
 namespace viam {
 namespace sdk {
 
-std::vector<unsigned char> string_to_bytes(const std::string& s) {
-    std::vector<unsigned char> bytes(s.begin(), s.end());
-    return bytes;
-};
-
-std::string bytes_to_string(const std::vector<unsigned char>& 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<time_pt>::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<std::chrono::seconds>(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<int32_t>(nano_part.count()));
-
-    return result;
+    result->set_seconds(sec_floor.count());
+    result->set_nanos(static_cast<int32_t>(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<google::protobuf::Timestamp>::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<std::chrono::microseconds>::operator()(std::chrono::microseconds duration,
+                                                     google::protobuf::Duration* proto) const {
+    namespace sc = std::chrono;
+
+    const sc::seconds seconds = sc::duration_cast<sc::seconds>(duration);
+    const sc::nanoseconds nanos = duration - seconds;
+
+    proto->set_nanos(static_cast<int32_t>(nanos.count()));
+    proto->set_seconds(seconds.count());
 }
 
-std::chrono::microseconds from_proto(const google::protobuf::Duration& proto) {
+std::chrono::microseconds from_proto<google::protobuf::Duration>::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<sc::microseconds>(seconds_part);
     sc::microseconds from_nanos = sc::duration_cast<sc::microseconds>(nanos_part);
@@ -80,16 +75,26 @@ std::chrono::microseconds from_proto(const google::protobuf::Duration& proto) {
     return from_seconds + from_nanos;
 }
 
-google::protobuf::Duration to_proto(std::chrono::microseconds duration) {
-    namespace sc = std::chrono;
+void to_proto<response_metadata>::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<sc::seconds>(duration);
-    const sc::nanoseconds nanos = duration - seconds;
+response_metadata from_proto<common::v1::ResponseMetadata>::operator()(
+    const common::v1::ResponseMetadata* proto) const {
+    return {v2::from_proto(proto->captured_at())};
+}
+
+}  // namespace proto_convert_details
+
+std::vector<unsigned char> string_to_bytes(const std::string& s) {
+    std::vector<unsigned char> bytes(s.begin(), s.end());
+    return bytes;
+}
 
-    google::protobuf::Duration proto;
-    proto.set_nanos(static_cast<int32_t>(nanos.count()));
-    proto.set_seconds(seconds.count());
-    return proto;
+std::string bytes_to_string(const std::vector<unsigned char>& b) {
+    std::string img_string(b.begin(), b.end());
+    return img_string;
 }
 
 void set_logger_severity_from_args(int argc, char** argv) {
@@ -101,14 +106,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");
-}
-
 std::string random_debug_key() {
     static const char alphanum[] = "abcdefghijklmnopqrstuvwxyz";
     static std::default_random_engine generator(
@@ -154,25 +151,35 @@ 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() {
+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_;
+    return wrapped_context_.get();
 }
 
 ClientContext::operator grpc::ClientContext*() {
-    return &wrapped_context_;
+    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) {

src/viam/sdk/common/utils.hpp

@@ -1,14 +1,33 @@
 #pragma once
 
+#include <memory>
+
 #include <boost/optional/optional.hpp>
 #include <grpcpp/client_context.h>
 
-#include <viam/api/common/v1/common.pb.h>
-
 #include <viam/sdk/common/proto_value.hpp>
 #include <viam/sdk/components/component.hpp>
 #include <viam/sdk/resource/resource_api.hpp>
 
+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<std::chrono::system_clock, std::chrono::
 
 struct response_metadata {
     time_pt captured_at;
-
-    static response_metadata from_proto(const viam::common::v1::ResponseMetadata& proto);
-    static viam::common::v1::ResponseMetadata to_proto(const response_metadata& metadata);
 };
 
 bool operator==(const response_metadata& lhs, const response_metadata& rhs);
 
-/// @brief convert a google::protobuf::Timestamp to time_pt
-time_pt timestamp_to_time_pt(const google::protobuf::Timestamp& timestamp);
+namespace proto_convert_details {
 
-/// @brief convert a time_pt to a google::protobuf::Timestamp.
-google::protobuf::Timestamp time_pt_to_timestamp(time_pt);
+template <>
+struct to_proto<time_pt> {
+    void operator()(time_pt, google::protobuf::Timestamp*) const;
+};
+
+template <>
+struct from_proto<google::protobuf::Timestamp> {
+    time_pt operator()(const google::protobuf::Timestamp*) const;
+};
+
+template <>
+struct to_proto<std::chrono::microseconds> {
+    void operator()(std::chrono::microseconds, google::protobuf::Duration*) const;
+};
+
+template <>
+struct from_proto<google::protobuf::Duration> {
+    std::chrono::microseconds operator()(const google::protobuf::Duration*) const;
+};
+
+template <>
+struct to_proto<response_metadata> {
+    void operator()(const response_metadata&, common::v1::ResponseMetadata*) const;
+};
+
+template <>
+struct from_proto<common::v1::ResponseMetadata> {
+    response_metadata operator()(const common::v1::ResponseMetadata*) const;
+};
+
+}  // namespace proto_convert_details
 
 std::vector<unsigned char> string_to_bytes(std::string const& s);
 std::string bytes_to_string(std::vector<unsigned char> 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
@@ -49,14 +90,19 @@ google::protobuf::Duration to_proto(std::chrono::microseconds duration);
 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_();
-    grpc::ClientContext wrapped_context_;
+    std::unique_ptr<grpc::ClientContext> wrapped_context_;
 };
 
 /// @brief Given a fully qualified resource name, returns remote name (or "" if no remote name

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();

src/viam/sdk/components/private/board_server.cpp

@@ -226,7 +226,7 @@ ::grpc::Status BoardServer::SetPowerMode(
     return make_service_helper<Board>(
         "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());

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;
 }
 

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);
     });
 }