RSDK-9549: Make RpcSubtype model api ResourceRPCSubtype (#349)

Author: lia-viam

src/viam/examples/modules/simple/client.cpp

@@ -1,3 +1,4 @@
+#include <iostream>
 #include <memory>
 #include <string>
 

src/viam/examples/motor/example_motor.cpp

@@ -1,7 +1,9 @@
+#include <unistd.h>
+
 #include <chrono>
 #include <fstream>
+#include <iostream>
 #include <string>
-#include <unistd.h>
 #include <vector>
 
 #include <viam/sdk/components/motor.hpp>

src/viam/sdk/common/utils.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <chrono>
 #include <memory>
 
 #include <boost/optional/optional.hpp>

src/viam/sdk/config/resource.cpp

@@ -95,19 +95,19 @@ const ProtoStruct& ResourceConfig::attributes() const {
 void ResourceConfig::fix_api() {
     if (this->api_.type_namespace().empty() && this->namespace__.empty()) {
         this->namespace__ = kRDK;
-        this->api_.set_namespace(kRDK);
+        this->api_.namespace_ = kRDK;
     } else if (this->api_.type_namespace().empty()) {
-        this->api_.set_namespace(this->namespace__);
+        api_.namespace_ = namespace__;
     } else {
         this->namespace__ = this->api_.type_namespace();
     }
 
     if (this->api_.resource_type().empty()) {
-        this->api_.set_resource_type(kComponent);
+        api_.resource_type_ = kComponent;
     }
 
     if (this->api_.resource_subtype().empty()) {
-        this->api_.set_resource_subtype(this->type_);
+        api_.resource_subtype_ = this->type_;
     } else if (this->type_.empty()) {
         this->type_ = this->api_.resource_subtype();
     }

src/viam/sdk/module/handler_map.cpp

@@ -3,7 +3,6 @@
 #include <memory>
 
 #include <boost/log/trivial.hpp>
-#include <google/protobuf/descriptor.h>
 
 #include <viam/api/common/v1/common.pb.h>
 #include <viam/api/module/v1/module.pb.h>
@@ -14,70 +13,68 @@
 namespace viam {
 namespace sdk {
 
-viam::module::v1::HandlerMap HandlerMap_::to_proto() const {
-    viam::module::v1::HandlerMap proto;
-    for (const auto& h : this->handles_) {
+void HandlerMap_::add_model(Model model, const RPCSubtype& subtype) {
+    handles_[subtype].push_back(std::move(model));
+}
+
+const std::unordered_map<RPCSubtype, std::vector<Model>>& HandlerMap_::handles() const {
+    return handles_;
+}
+
+std::ostream& operator<<(std::ostream& os, const HandlerMap_& hm) {
+    for (const auto& kv : hm.handles()) {
+        os << "API: " << kv.first.api().to_string() << '\n';
+        for (const Model& model : kv.second) {
+            os << "\tModel: " << model.to_string() << '\n';
+        }
+    }
+    return os;
+}
+
+namespace proto_convert_details {
+
+void to_proto<HandlerMap_>::operator()(const HandlerMap_& self,
+                                       module::v1::HandlerMap* proto) const {
+    for (const auto& h : self.handles()) {
         viam::module::v1::HandlerDefinition hd;
         for (const auto& model : h.second) {
-            const std::string m = model.to_string();
-            *hd.mutable_models()->Add() = m;
+            *hd.mutable_models()->Add() = model.to_string();
         }
+
         viam::robot::v1::ResourceRPCSubtype rpc_subtype;
-        const Name name(h.first.api(), "", "");
-        const viam::common::v1::ResourceName resource_name = v2::to_proto(name);
-        *rpc_subtype.mutable_subtype() = resource_name;
+
+        *rpc_subtype.mutable_subtype() = v2::to_proto(Name(h.first.api(), "", ""));
         *rpc_subtype.mutable_proto_service() = h.first.proto_service_name();
         *hd.mutable_subtype() = rpc_subtype;
-        *proto.add_handlers() = hd;
-    }
-
-    return proto;
-};
 
-HandlerMap_::HandlerMap_() {}
+        *proto->add_handlers() = hd;
+    }
+}
 
-// NOLINTNEXTLINE(readability-const-return-type)
-const HandlerMap_ HandlerMap_::from_proto(const viam::module::v1::HandlerMap& proto) {
+HandlerMap_ from_proto<module::v1::HandlerMap>::operator()(
+    const module::v1::HandlerMap* proto) const {
     HandlerMap_ hm;
 
     const google::protobuf::RepeatedPtrField<viam::module::v1::HandlerDefinition>& handlers =
-        proto.handlers();
+        proto->handlers();
 
     for (const auto& handler : handlers) {
-        std::vector<Model> models;
         const viam::common::v1::ResourceName name = handler.subtype().subtype();
         const API api(name.namespace_(), name.type(), name.subtype());
-        const google::protobuf::DescriptorPool* pool =
-            google::protobuf::DescriptorPool::generated_pool();
-        const google::protobuf::ServiceDescriptor* sd = pool->FindServiceByName(name.type());
-        const RPCSubtype handle(api, *sd);
+        const RPCSubtype handle(api);
         for (const auto& mod : handler.models()) {
             try {
-                const Model model = Model::from_str(mod);
-                models.push_back(model);
-            } catch (std::string error) {  // NOLINT
-                BOOST_LOG_TRIVIAL(error) << "Error processing model " + mod;
+                hm.add_model(Model::from_str(mod), handle);
+            } catch (const std::exception& ex) {
+                BOOST_LOG_TRIVIAL(error) << "Error " << ex.what() << " processing model " + mod;
             }
         }
-        hm.handles_.emplace(handle, models);
     }
 
     return hm;
 }
 
-void HandlerMap_::add_model(Model model, const RPCSubtype& subtype) {
-    handles_[subtype].push_back(std::move(model));
-}
-
-std::ostream& operator<<(std::ostream& os, const HandlerMap_& hm) {
-    for (const auto& kv : hm.handles_) {
-        os << "API: " << kv.first.api().to_string() << '\n';
-        for (const Model& model : kv.second) {
-            os << "\tModel: " << model.to_string() << '\n';
-        }
-    }
-    return os;
-}
+}  // namespace proto_convert_details
 
 }  // namespace sdk
 }  // namespace viam

src/viam/sdk/module/handler_map.hpp

@@ -1,24 +1,47 @@
 #pragma once
 
-#include <viam/api/module/v1/module.pb.h>
-
+#include <viam/sdk/common/proto_convert.hpp>
 #include <viam/sdk/resource/resource.hpp>
 
+namespace viam {
+namespace module {
+namespace v1 {
+
+class HandlerMap;
+
+}  // namespace v1
+}  // namespace module
+}  // namespace viam
+
 namespace viam {
 namespace sdk {
 
 class HandlerMap_ {
    public:
-    HandlerMap_();
+    HandlerMap_() = default;
+
     void add_model(Model model, const RPCSubtype& subtype);
 
-    viam::module::v1::HandlerMap to_proto() const;
-    static const HandlerMap_ from_proto(const viam::module::v1::HandlerMap& proto);
-    friend std::ostream& operator<<(std::ostream& os, const HandlerMap_& hm);
+    const std::unordered_map<RPCSubtype, std::vector<Model>>& handles() const;
 
    private:
     std::unordered_map<RPCSubtype, std::vector<Model>> handles_;
 };
 
+std::ostream& operator<<(std::ostream& os, const HandlerMap_& hm);
+
+namespace proto_convert_details {
+
+template <>
+struct to_proto<HandlerMap_> {
+    void operator()(const HandlerMap_&, module::v1::HandlerMap*) const;
+};
+
+template <>
+struct from_proto<module::v1::HandlerMap> {
+    HandlerMap_ operator()(const module::v1::HandlerMap*) const;
+};
+
+}  // namespace proto_convert_details
 }  // namespace sdk
 }  // namespace viam

src/viam/sdk/module/service.cpp

@@ -55,7 +55,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service {
         const std::lock_guard<std::mutex> lock(parent.lock_);
 
         std::shared_ptr<Resource> res;
-        const Dependencies deps = parent.get_dependencies_(request->dependencies(), cfg.name());
+        const Dependencies deps = parent.get_dependencies_(&request->dependencies(), cfg.name());
         const std::shared_ptr<const ModelRegistration> reg =
             Registry::lookup_model(cfg.api(), cfg.model());
         if (reg) {
@@ -81,7 +81,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service {
         const viam::app::v1::ComponentConfig& proto = request->config();
         ResourceConfig cfg = v2::from_proto(proto);
 
-        const Dependencies deps = parent.get_dependencies_(request->dependencies(), cfg.name());
+        const Dependencies deps = parent.get_dependencies_(&request->dependencies(), cfg.name());
 
         auto resource_server = parent.server_->lookup_resource_server(cfg.api());
         if (!resource_server) {
@@ -179,7 +179,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service {
                          const ::viam::module::v1::ReadyRequest* request,
                          ::viam::module::v1::ReadyResponse* response) override {
         const std::lock_guard<std::mutex> lock(parent.lock_);
-        const viam::module::v1::HandlerMap hm = parent.module_->handles().to_proto();
+        const viam::module::v1::HandlerMap hm = v2::to_proto(parent.module_->handles());
         *response->mutable_handlermap() = hm;
         parent.parent_addr_ = request->parent_address();
         response->set_ready(parent.module_->ready());
@@ -188,10 +188,10 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service {
 };
 
 Dependencies ModuleService::get_dependencies_(
-    google::protobuf::RepeatedPtrField<std::string> const& proto,
+    google::protobuf::RepeatedPtrField<std::string> const* proto,
     std::string const& resource_name) {
     Dependencies deps;
-    for (const auto& dep : proto) {
+    for (const auto& dep : *proto) {
         auto dep_name = Name::from_string(dep);
         const std::shared_ptr<Resource> dep_resource = get_parent_resource_(dep_name);
         if (!dep_resource) {
@@ -276,12 +276,10 @@ void ModuleService::add_model_from_registry_inlock_(API api,
     const std::shared_ptr<const ResourceServerRegistration> creator =
         Registry::lookup_resource_server(api);
     std::string name;
-    const google::protobuf::ServiceDescriptor* sd = nullptr;
     if (creator && creator->service_descriptor()) {
         name = creator->service_descriptor()->full_name();
-        sd = creator->service_descriptor();
     }
-    const RPCSubtype rpc_subtype(std::move(api), name, *sd);
+    const RPCSubtype rpc_subtype(std::move(api), name);
     module_->mutable_handles().add_model(std::move(model), rpc_subtype);
 };
 

src/viam/sdk/module/service.hpp

@@ -8,6 +8,15 @@
 #include <viam/sdk/resource/resource.hpp>
 #include <viam/sdk/rpc/server.hpp>
 
+namespace google {
+namespace protobuf {
+
+template <typename>
+class RepeatedPtrField;
+
+}  // namespace protobuf
+}  // namespace google
+
 namespace viam {
 namespace sdk {
 
@@ -51,7 +60,7 @@ class ModuleService {
     friend ModuleService::ServiceImpl;
 
     void add_model_from_registry_inlock_(API api, Model model, const std::lock_guard<std::mutex>&);
-    Dependencies get_dependencies_(google::protobuf::RepeatedPtrField<std::string> const& proto,
+    Dependencies get_dependencies_(google::protobuf::RepeatedPtrField<std::string> const* proto,
                                    std::string const& resource_name);
     std::shared_ptr<Resource> get_parent_resource_(const Name& name);
 

src/viam/sdk/resource/resource.cpp

@@ -16,11 +16,10 @@ std::string Resource::name() const {
 }
 
 Name Resource::get_resource_name(const std::string& type) const {
-    auto api_ = api();
-    api_.set_resource_type(type);
-
     auto name_parts = long_name_to_remote_and_short(name_);
-    return {api_, name_parts.first, name_parts.second};
+    return {API(api().type_namespace(), type, api().resource_subtype()),
+            name_parts.first,
+            name_parts.second};
 }
 
 Name Resource::get_resource_name() const {