Draft: RSDK-9373 unify proto value conversions

src/viam/sdk/common/client_helper.hpp

@@ -59,7 +59,7 @@ class ClientHelper {
             ProtoValue value = key->second;
             debug_key_ = *value.get<std::string>();
         }
-        *request_.mutable_extra() = map_to_struct(extra);
+        *request_.mutable_extra() = v2::to_proto(extra);
         return with(std::forward<RequestSetupCallable>(rsc));
     }
 

src/viam/sdk/common/proto_value.cpp

@@ -43,37 +43,6 @@ ProtoValue::ProtoValue(ProtoValue&& other) noexcept(proto_value_details::all_mov
 ProtoValue::ProtoValue(const ProtoValue& other)
     : vtable_(other.vtable_), self_(other.self_, other.vtable_) {}
 
-ProtoValue::ProtoValue(const Value* value)  // NOLINT(misc-no-recursion)
-    : ProtoValue([](const Value& v) {       // NOLINT(misc-no-recursion)
-          switch (v.kind_case()) {
-              case Value::KindCase::kBoolValue: {
-                  return ProtoValue(v.bool_value());
-              }
-              case Value::KindCase::kStringValue: {
-                  return ProtoValue(v.string_value());
-              }
-              case Value::KindCase::kNumberValue: {
-                  return ProtoValue(v.number_value());
-              }
-              case Value::KindCase::kListValue: {
-                  ProtoList vec;
-                  vec.reserve(v.list_value().values_size());
-                  for (const Value& list_val : v.list_value().values()) {
-                      vec.push_back(ProtoValue::from_proto(list_val));
-                  }
-
-                  return ProtoValue(std::move(vec));
-              }
-              case Value::KindCase::kStructValue: {
-                  return ProtoValue(struct_to_map(v.struct_value()));
-              }
-              case Value::KindCase::KIND_NOT_SET:
-              case Value::KindCase::kNullValue:
-              default:
-                  return ProtoValue(nullptr);
-          }
-      }(*value)) {}
-
 ProtoValue& ProtoValue::operator=(ProtoValue&& other) noexcept(
     proto_value_details::all_moves_noexcept{}) {
     ProtoValue(std::move(other)).swap(*this);
@@ -98,13 +67,6 @@ void ProtoValue::swap(ProtoValue& other) noexcept(proto_value_details::all_moves
     std::swap(vtable_, other.vtable_);
 }
 
-template <typename Val>
-ProtoValue ProtoValue::from_proto(const Val& v) {  // NOLINT(misc-no-recursion)
-    return ProtoValue(&v);
-}
-
-template ProtoValue ProtoValue::from_proto(const Value&);
-
 ProtoValue::Kind ProtoValue::kind() const {
     return vtable_.kind();
 }
@@ -182,8 +144,8 @@ void ProtoValue::model<T>::move(void* self, void* dest) {
 }
 
 template <typename T>
-void ProtoValue::model<T>::to_proto(void const* self, google::protobuf::Value* v) {
-    viam::sdk::to_proto(static_cast<model const*>(self)->data, v);
+void ProtoValue::model<T>::to_value(void const* self, google::protobuf::Value* v) {
+    viam::sdk::proto_value_details::to_value(static_cast<model const*>(self)->data, v);
 }
 
 template <typename T>
@@ -254,53 +216,94 @@ void ProtoValue::storage::destruct(const ProtoValue::vtable& vtab) noexcept {
     vtab.dtor(this->get());
 }
 
-void to_proto(std::nullptr_t, Value* v) {
+namespace proto_value_details {
+
+void to_value(std::nullptr_t, Value* v) {
     v->set_null_value(::google::protobuf::NULL_VALUE);
 }
 
-void to_proto(bool b, Value* v) {
+void to_value(bool b, Value* v) {
     v->set_bool_value(b);
 }
 
-void to_proto(double d, Value* v) {
+void to_value(double d, Value* v) {
     v->set_number_value(d);
 }
 
-void to_proto(std::string s, Value* v) {
+void to_value(std::string s, Value* v) {
     v->set_string_value(std::move(s));
 }
 
-void to_proto(const ProtoList& vec, Value* v) {
+void to_value(const ProtoList& vec, Value* v) {
     ::google::protobuf::ListValue l;
     for (const auto& val : vec) {
-        *l.add_values() = to_proto(val);
+        *l.add_values() = v2::to_proto(val);
     }
     *(v->mutable_list_value()) = std::move(l);
 }
 
-void to_proto(const ProtoStruct& m, Value* v) {
-    Struct s;
-    map_to_struct(m, &s);
-
-    *(v->mutable_struct_value()) = std::move(s);
+void to_value(const ProtoStruct& m, Value* v) {
+    *(v->mutable_struct_value()) = v2::to_proto(m);
+}
+
+}  // namespace proto_value_details
+
+namespace proto_convert_details {
+
+void to_proto<ProtoValue>::operator()(const ProtoValue& self, google::protobuf::Value* v) const {
+    self.vtable_.to_value(self.self_.get(), v);
+}
+
+ProtoValue from_proto<google::protobuf::Value>::operator()(
+    const google::protobuf::Value* v) const {  // NOLINT(misc-no-recursion)
+    switch (v->kind_case()) {
+        case Value::KindCase::kBoolValue: {
+            return ProtoValue(v->bool_value());
+        }
+        case Value::KindCase::kStringValue: {
+            return ProtoValue(v->string_value());
+        }
+        case Value::KindCase::kNumberValue: {
+            return ProtoValue(v->number_value());
+        }
+        case Value::KindCase::kListValue: {
+            ProtoList vec;
+            vec.reserve(v->list_value().values_size());
+            for (const Value& list_val : v->list_value().values()) {
+                vec.push_back(v2::from_proto(list_val));
+            }
+
+            return ProtoValue(std::move(vec));
+        }
+        case Value::KindCase::kStructValue: {
+            return ProtoValue(v2::from_proto(v->struct_value()));
+        }
+        case Value::KindCase::KIND_NOT_SET:
+        case Value::KindCase::kNullValue:
+        default:
+            return ProtoValue(nullptr);
+    }
 }
 
-void to_proto(const ProtoValue& t, Value* v) {
-    t.vtable_.to_proto(t.self_.get(), v);
+void to_proto<ProtoStruct>::operator()(const ProtoStruct& self, google::protobuf::Struct* s) const {
+    for (const auto& kv : self) {
+        s->mutable_fields()->insert(
+            google::protobuf::MapPair<std::string, Value>(kv.first, v2::to_proto(kv.second)));
+    }
 }
 
-void struct_to_map(Struct const* s, ProtoStruct& map) {  // NOLINT(misc-no-recursion)
+ProtoStruct from_proto<google::protobuf::Struct>::operator()(
+    const google::protobuf::Struct* s) const {  // NOLINT(misc-no-recursion)
+    ProtoStruct result;
+
     for (const auto& val : s->fields()) {
-        map.emplace(val.first, ProtoValue::from_proto(val.second));
+        result.emplace(val.first, v2::from_proto(val.second));
     }
-}
 
-void map_to_struct(const ProtoStruct& m, Struct* s) {
-    for (const auto& kv : m) {
-        s->mutable_fields()->insert(
-            google::protobuf::MapPair<std::string, Value>(kv.first, to_proto(kv.second)));
-    }
+    return result;
 }
 
+}  // namespace proto_convert_details
+
 }  // namespace sdk
 }  // namespace viam

src/viam/sdk/common/proto_value.hpp

@@ -7,6 +7,8 @@
 #include <utility>
 #include <vector>
 
+#include <viam/sdk/common/proto_convert.hpp>
+
 namespace google {
 namespace protobuf {
 
@@ -52,6 +54,8 @@ struct all_moves_noexcept
 /// definition.
 class ProtoValue {
    public:
+    friend proto_convert_details::to_proto<ProtoValue>;
+
     /// @brief Type discriminator constants for possible values stored in a ProtoValue.
     enum Kind { k_null = 0, k_bool = 1, k_double = 2, k_string = 3, k_list = 4, k_struct = 5 };
 
@@ -110,15 +114,6 @@ class ProtoValue {
 
     void swap(ProtoValue& other) noexcept(proto_value_details::all_moves_noexcept{});
 
-    /// @brief Construct from proto value
-    /// @note This method is trivially templated to insulate google::protobuf::Value from our
-    /// API/ABI. It is meant to be called with no template parameters in a translation unit which
-    /// includes <google/protobuf/struct.pb.h>
-    template <typename Value = google::protobuf::Value>
-    static ProtoValue from_proto(const Value& v);  // NOLINT(misc-no-recursion)
-
-    friend void to_proto(const ProtoValue& t, google::protobuf::Value* v);
-
     /// @name Value access API
     ///@{
 
@@ -178,7 +173,7 @@ class ProtoValue {
         void (*dtor)(void*);
         void (*copy)(void const*, void*);
         void (*move)(void*, void*);
-        void (*to_proto)(void const*, google::protobuf::Value*);
+        void (*to_value)(void const*, google::protobuf::Value*);
         Kind (*kind)();
         bool (*equal_to)(void const*, void const*, const vtable&);
     };
@@ -199,13 +194,13 @@ class ProtoValue {
         // non-noexcept pointer anyway
         static void move(void* self, void* dest);
 
-        static void to_proto(void const* self, google::protobuf::Value* v);
+        static void to_value(void const* self, google::protobuf::Value* v);
 
         static Kind kind() noexcept;
 
         static bool equal_to(void const* self, void const* other, const vtable& other_vtable);
 
-        static constexpr vtable vtable_{dtor, copy, move, to_proto, kind, equal_to};
+        static constexpr vtable vtable_{dtor, copy, move, to_value, kind, equal_to};
         T data;
     };
 
@@ -320,55 +315,39 @@ extern template std::string&& ProtoValue::get_unchecked<std::string>() &&;
 extern template ProtoList&& ProtoValue::get_unchecked<ProtoList>() &&;
 extern template ProtoStruct&& ProtoValue::get_unchecked<ProtoStruct>() &&;
 
-void to_proto(std::nullptr_t, google::protobuf::Value* v);
-void to_proto(bool b, google::protobuf::Value* v);
-void to_proto(double d, google::protobuf::Value* v);
-void to_proto(std::string s, google::protobuf::Value* v);
-void to_proto(const ProtoList& vec, google::protobuf::Value* v);
-void to_proto(const ProtoStruct& m, google::protobuf::Value* v);
-void to_proto(const ProtoValue& t, google::protobuf::Value* v);
-
-void struct_to_map(google::protobuf::Struct const* s, ProtoStruct& m);
-void map_to_struct(const ProtoStruct& m, google::protobuf::Struct* s);
-
-/// @brief Convert a type to a google::protobuf::Value.
-/// @note This method is trivially templated to insulate google::protobuf::Value from our
-/// API/ABI. It is meant to be called with no template parameters in a translation unit which
-/// includes <google/protobuf/struct.pb.h>
-template <typename Value = google::protobuf::Value>
-Value to_proto(const ProtoValue& proto_value) {
-    Value v;
-    to_proto(proto_value, &v);
-
-    return v;
-}
+namespace proto_convert_details {
 
-/// @brief Convert a google::protobuf::Struct to a ProtoStruct.
-/// @note This method is trivially templated to insulate google::protobuf::Struct from our
-/// API/ABI. It is meant to be called with no template parameters in a translation unit which
-/// includes <google/protobuf/struct.pb.h>
-template <typename Struct = google::protobuf::Struct>
-ProtoStruct struct_to_map(const Struct& s) {
-    ProtoStruct result;
-    struct_to_map(&s, result);
+template <>
+struct to_proto<ProtoValue> {
+    void operator()(const ProtoValue&, google::protobuf::Value*) const;
+};
 
-    return result;
-}
+template <>
+struct to_proto<ProtoStruct> {
+    void operator()(const ProtoStruct&, google::protobuf::Struct*) const;
+};
 
-/// @brief Convert a ProtoStruct to a google::protobuf::Struct.
-/// @note This method is trivially templated to insulate google::protobuf::Struct from our
-/// API/ABI. It is meant to be called with no template parameters in a translation unit which
-/// includes <google/protobuf/struct.pb.h>
-template <typename Struct = google::protobuf::Struct>
-Struct map_to_struct(const ProtoStruct& m) {
-    Struct s;
-    map_to_struct(m, &s);
+template <>
+struct from_proto<google::protobuf::Value> {
+    ProtoValue operator()(const google::protobuf::Value*) const;
+};
 
-    return s;
-}
+template <>
+struct from_proto<google::protobuf::Struct> {
+    ProtoStruct operator()(const google::protobuf::Struct*) const;
+};
+
+}  // namespace proto_convert_details
 
 namespace proto_value_details {
 
+void to_value(std::nullptr_t, google::protobuf::Value* v);
+void to_value(bool b, google::protobuf::Value* v);
+void to_value(double d, google::protobuf::Value* v);
+void to_value(std::string s, google::protobuf::Value* v);
+void to_value(const ProtoList& vec, google::protobuf::Value* v);
+void to_value(const ProtoStruct& m, google::protobuf::Value* v);
+
 template <typename T>
 struct kind {};
 

src/viam/sdk/common/service_helper.hpp

@@ -49,7 +49,7 @@ class ServiceHelper : public ServiceHelperBase {
     }
 
     auto getExtra() const {
-        return request_->has_extra() ? struct_to_map(request_->extra()) : ProtoStruct{};
+        return request_->has_extra() ? v2::from_proto(request_->extra()) : ProtoStruct{};
     }
 
    private:

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

@@ -83,8 +83,8 @@ void ArmClient::stop(const ProtoStruct& extra) {
 
 ProtoStruct ArmClient::do_command(const ProtoStruct& command) {
     return make_client_helper(this, *stub_, &StubType::DoCommand)
-        .with([&](auto& request) { *request.mutable_command() = map_to_struct(command); })
-        .invoke([](auto& response) { return struct_to_map(response.result()); });
+        .with([&](auto& request) { *request.mutable_command() = v2::to_proto(command); })
+        .invoke([](auto& response) { return v2::from_proto(response.result()); });
 }
 
 Arm::KinematicsData ArmClient::get_kinematics(const ProtoStruct& extra) {

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

@@ -98,8 +98,8 @@ ::grpc::Status ArmServer::DoCommand(::grpc::ServerContext*,
                                     const ::viam::common::v1::DoCommandRequest* request,
                                     ::viam::common::v1::DoCommandResponse* response) noexcept {
     return make_service_helper<Arm>("ArmServer::DoCommand", this, request)([&](auto&, auto& arm) {
-        const ProtoStruct result = arm->do_command(struct_to_map(request->command()));
-        *response->mutable_result() = map_to_struct(result);
+        const ProtoStruct result = arm->do_command(v2::from_proto(request->command()));
+        *response->mutable_result() = v2::to_proto(result);
     });
 }
 

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

@@ -99,8 +99,8 @@ Base::properties BaseClient::get_properties(const ProtoStruct& extra) {
 
 ProtoStruct BaseClient::do_command(const ProtoStruct& command) {
     return make_client_helper(this, *stub_, &StubType::DoCommand)
-        .with([&](auto& request) { *request.mutable_command() = map_to_struct(command); })
-        .invoke([](auto& response) { return struct_to_map(response.result()); });
+        .with([&](auto& request) { *request.mutable_command() = v2::to_proto(command); })
+        .invoke([](auto& response) { return v2::from_proto(response.result()); });
 }
 
 }  // namespace impl

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

@@ -106,8 +106,8 @@ ::grpc::Status BaseServer::DoCommand(grpc::ServerContext*,
                                      viam::common::v1::DoCommandResponse* response) noexcept {
     return make_service_helper<Base>(
         "BaseServer::DoCommand", this, request)([&](auto&, auto& base) {
-        const ProtoStruct result = base->do_command(struct_to_map(request->command()));
-        *response->mutable_result() = map_to_struct(result);
+        const ProtoStruct result = base->do_command(v2::from_proto(request->command()));
+        *response->mutable_result() = v2::to_proto(result);
     });
 }