From 0c1785b9e678dd9121ef86c374955639dccc9654 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Thu, 25 Jan 2024 15:10:23 -0500 Subject: [PATCH 01/20] minor cleanup --- .github/workflows/docs.yml | 4 ++-- src/viam/sdk/module/service.cpp | 2 +- src/viam/sdk/registry/registry.hpp | 1 + src/viam/sdk/rpc/server.cpp | 6 ++++-- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 756a86481..3961ea70c 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -17,10 +17,10 @@ jobs: steps: - name: Checkout uses: actions/checkout@v3 - + - name: install Doxygen uses: ssciwr/doxygen-install@v1 - + - name: create path run: mkdir -p etc/docs/api/current diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 257e411c2..ccae04204 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -243,7 +243,7 @@ void ModuleService::serve() { server_->start(); BOOST_LOG_TRIVIAL(info) << "Module listening on " << module_->addr(); - BOOST_LOG_TRIVIAL(info) << "Module handles the following API/model pairs: " << std::endl + BOOST_LOG_TRIVIAL(info) << "Module handles the following API/model pairs:\n" << module_->handles(); signal_manager_.wait(); diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index 969900215..ccd011967 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -31,6 +31,7 @@ class ResourceRegistration { public: virtual ~ResourceRegistration(); + // CR erodkin: is this necessary at all? how do we use this? /// @brief Add `Reconfigure` functionality to a resource. std::function(std::shared_ptr, Name)> create_reconfigurable; diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 60954bf2d..bca7c1711 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -14,7 +14,7 @@ Server::~Server() { void Server::register_service(grpc::Service* service) { if (!builder_) { - throw "Cannot register a new service after the server has started"; + throw std::runtime_error("Cannot register a new service after the server has started"); } builder_->RegisterService(service); @@ -25,6 +25,8 @@ void Server::start() { throw std::runtime_error("Attempted to start server that was already running"); } + // CR erodkin: this is supposed to be called at static initialization. We can get close + // enough (I suspect) by putting this into the `TheOneAndOnly` call. grpc::reflection::InitProtoReflectionServerBuilderPlugin(); server_ = builder_->BuildAndStart(); builder_ = nullptr; @@ -33,7 +35,7 @@ void Server::start() { void Server::add_listening_port(std::string address, std::shared_ptr creds) { if (!builder_) { - throw "Cannot add a listening port after server has started"; + throw std::runtime_error("Cannot add a listening port after server has started"); } if (!creds) { From 17eedd321a0a53069f0a666b65f0e18a1cc3939d Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 29 Jan 2024 11:03:34 -0500 Subject: [PATCH 02/20] refactor ResourceRegistration2 --- src/viam/sdk/common/viam.cpp | 36 ++++++++++++ src/viam/sdk/common/viam.hpp | 16 +++++ .../movement_sensor/movement_sensor.cpp | 15 ++--- .../components/power_sensor/power_sensor.cpp | 15 ++--- src/viam/sdk/components/sensor/sensor.cpp | 12 +--- src/viam/sdk/registry/registry.cpp | 10 ++++ src/viam/sdk/registry/registry.hpp | 58 ++++++++----------- 7 files changed, 97 insertions(+), 65 deletions(-) create mode 100644 src/viam/sdk/common/viam.cpp create mode 100644 src/viam/sdk/common/viam.hpp diff --git a/src/viam/sdk/common/viam.cpp b/src/viam/sdk/common/viam.cpp new file mode 100644 index 000000000..316f9548c --- /dev/null +++ b/src/viam/sdk/common/viam.cpp @@ -0,0 +1,36 @@ +#include "viam/sdk/components/base/base.hpp" +#include "viam/sdk/components/board/board.hpp" +#include "viam/sdk/components/camera/camera.hpp" +#include "viam/sdk/components/encoder/encoder.hpp" +#include "viam/sdk/components/motor/motor.hpp" +#include "viam/sdk/components/movement_sensor/movement_sensor.hpp" +#include "viam/sdk/components/power_sensor/power_sensor.hpp" +#include "viam/sdk/components/sensor/sensor.hpp" +#include "viam/sdk/components/servo/servo.hpp" +#include "viam/sdk/registry/registry.hpp" +#include +#include + +namespace viam { +namespace sdk { + +// CR erodkin: probably we can't make this work sadly. +typedef boost::mpl::vector resources; + +ViamInit::ViamInit() { + Registry::register_resource(API::get(), Base::resource_registration()); + Registry::register_resource(API::get(), Board::resource_registration()); + Registry::register_resource(API::get(), Camera::resource_registration()); + Registry::register_resource(API::get(), Encoder::resource_registration()); + Registry::register_resource(API::get(), + GenericComponent::resource_registration()); + Registry::register_resource(API::get(), Motor::resource_registration()); + Registry::register_resource(API::get(), + MovementSensor::resource_registration()); + Registry::register_resource(API::get(), PowerSensor::resource_registration()); + Registry::register_resource(API::get(), Sensor::resource_registration()); + Registry::register_resource(API::get(), Servo::resource_registration()); +} + +} // namespace sdk +} // namespace viam diff --git a/src/viam/sdk/common/viam.hpp b/src/viam/sdk/common/viam.hpp new file mode 100644 index 000000000..2418f85a4 --- /dev/null +++ b/src/viam/sdk/common/viam.hpp @@ -0,0 +1,16 @@ +#pragma once + +namespace viam { +namespace sdk { + +class ViamInit { + public: + ViamInit(); + ~ViamInit(); + + private: + static bool initialized_; +}; + +} // namespace sdk +} // namespace viam diff --git a/src/viam/sdk/components/movement_sensor/movement_sensor.cpp b/src/viam/sdk/components/movement_sensor/movement_sensor.cpp index a04105866..e93e7c794 100644 --- a/src/viam/sdk/components/movement_sensor/movement_sensor.cpp +++ b/src/viam/sdk/components/movement_sensor/movement_sensor.cpp @@ -105,18 +105,11 @@ bool operator==(const MovementSensor::properties& lhs, const MovementSensor::pro } namespace { -class MovementSensorRegistration final - : public ResourceRegistration2 { - public: - using ResourceRegistration2::ResourceRegistration2; -}; - bool init() { - Registry::register_resource(API::get(), - MovementSensorRegistration::resource_registration()); + Registry::register_resource( + API::get()); return true; }; diff --git a/src/viam/sdk/components/power_sensor/power_sensor.cpp b/src/viam/sdk/components/power_sensor/power_sensor.cpp index c052e9127..70eacff35 100644 --- a/src/viam/sdk/components/power_sensor/power_sensor.cpp +++ b/src/viam/sdk/components/power_sensor/power_sensor.cpp @@ -59,18 +59,11 @@ bool operator==(const PowerSensor::current& lhs, const PowerSensor::current& rhs } namespace { -class PowerSensorRegistration final - : public ResourceRegistration2 { - public: - using ResourceRegistration2::ResourceRegistration2; -}; - bool init() { - Registry::register_resource(API::get(), - PowerSensorRegistration::resource_registration()); + Registry::register_resource( + API::get()); return true; }; diff --git a/src/viam/sdk/components/sensor/sensor.cpp b/src/viam/sdk/components/sensor/sensor.cpp index 35e4886bd..717c09c81 100644 --- a/src/viam/sdk/components/sensor/sensor.cpp +++ b/src/viam/sdk/components/sensor/sensor.cpp @@ -20,17 +20,9 @@ API API::traits::api() { } namespace { -class SensorRegistration final - : public ResourceRegistration2 { - public: - using ResourceRegistration2::ResourceRegistration2; -}; - bool init() { - Registry::register_resource(API::get(), SensorRegistration::resource_registration()); + Registry::register_resource( + API::get()); return true; }; diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 2203105e5..37492497e 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -72,6 +72,16 @@ std::shared_ptr Registry::lookup_resource(API api) { return apis_.at(api); } +const google::protobuf::ServiceDescriptor* Registry::get_service_descriptor_( + const char* service_full_name) { + const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); + const google::protobuf::ServiceDescriptor* sd = p->FindServiceByName(service_full_name); + if (!sd) { + throw std::runtime_error("Unable to get service descriptor"); + } + return sd; +} + std::unordered_map> Registry::registered_models() { std::unordered_map> registry; for (auto& resource : resources_) { diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index ccd011967..cc32ba943 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -59,39 +59,6 @@ class ResourceRegistration { const google::protobuf::ServiceDescriptor* service_descriptor_; }; -// TODO(RSDK-3030): Potentially make ResourceRegistration2 the one and only -// form of resource registration. -template -class ResourceRegistration2 : public ResourceRegistration { - public: - using ResourceRegistration::ResourceRegistration; - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override { - auto rs = std::make_shared(manager); - server.register_service(rs.get()); - return rs; - } - - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override { - return std::make_shared(std::move(name), std::move(chan)); - } - - static std::shared_ptr resource_registration() { - const google::protobuf::DescriptorPool* p = - google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(ProtoServiceT::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor"); - } - return std::make_shared(sd); - } -}; - /// @class ModelRegistration /// @brief Information about a registered model, including a constructor and config validator. class ModelRegistration { @@ -159,6 +126,29 @@ class Registry { /// @return a `shared_ptr` to the resource's registration data. static std::shared_ptr lookup_model(API api, Model model); + template + static void register_resource(API api) { + class ResourceRegistration2 final : public ResourceRegistration { + public: + using ResourceRegistration::ResourceRegistration; + std::shared_ptr create_resource_server( + std::shared_ptr manager, Server& server) override { + auto rs = std::make_shared(manager); + server.register_service(rs.get()); + return rs; + } + + std::shared_ptr create_rpc_client( + std::string name, std::shared_ptr chan) override { + return std::make_shared(std::move(name), std::move(chan)); + } + }; + + const google::protobuf::ServiceDescriptor* sd = + get_service_descriptor_(ProtoServiceT::service_full_name()); + Registry::register_resource(api, std::make_shared(sd)); + } + /// @brief Register an api. /// @param api The api to be registered. /// @param resource_registration `ResourceRegistration` with resource functionality. @@ -175,6 +165,8 @@ class Registry { static std::unordered_map> registered_models(); private: + static const google::protobuf::ServiceDescriptor* get_service_descriptor_( + const char* service_full_name); static std::unordered_map> resources_; static std::unordered_map> apis_; }; From 78a677a133fa28b54902042e406abdfe7549bb68 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 29 Jan 2024 11:11:54 -0500 Subject: [PATCH 03/20] start building out init --- src/viam/sdk/common/viam.cpp | 34 ++++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/src/viam/sdk/common/viam.cpp b/src/viam/sdk/common/viam.cpp index 316f9548c..d509f1ef9 100644 --- a/src/viam/sdk/common/viam.cpp +++ b/src/viam/sdk/common/viam.cpp @@ -1,11 +1,26 @@ +#include "component/base/v1/base.grpc.pb.h" +#include "component/board/v1/board.grpc.pb.h" +#include "component/camera/v1/camera.grpc.pb.h" +#include "component/encoder/v1/encoder.grpc.pb.h" +#include "component/sensor/v1/sensor.grpc.pb.h" #include "viam/sdk/components/base/base.hpp" +#include "viam/sdk/components/base/client.hpp" +#include "viam/sdk/components/base/server.hpp" #include "viam/sdk/components/board/board.hpp" +#include "viam/sdk/components/board/client.hpp" +#include "viam/sdk/components/board/server.hpp" #include "viam/sdk/components/camera/camera.hpp" +#include "viam/sdk/components/camera/client.hpp" +#include "viam/sdk/components/camera/server.hpp" +#include "viam/sdk/components/encoder/client.hpp" #include "viam/sdk/components/encoder/encoder.hpp" +#include "viam/sdk/components/encoder/server.hpp" #include "viam/sdk/components/motor/motor.hpp" #include "viam/sdk/components/movement_sensor/movement_sensor.hpp" #include "viam/sdk/components/power_sensor/power_sensor.hpp" +#include "viam/sdk/components/sensor/client.hpp" #include "viam/sdk/components/sensor/sensor.hpp" +#include "viam/sdk/components/sensor/server.hpp" #include "viam/sdk/components/servo/servo.hpp" #include "viam/sdk/registry/registry.hpp" #include @@ -14,21 +29,24 @@ namespace viam { namespace sdk { -// CR erodkin: probably we can't make this work sadly. -typedef boost::mpl::vector resources; - ViamInit::ViamInit() { - Registry::register_resource(API::get(), Base::resource_registration()); - Registry::register_resource(API::get(), Board::resource_registration()); - Registry::register_resource(API::get(), Camera::resource_registration()); - Registry::register_resource(API::get(), Encoder::resource_registration()); + Registry::register_resource( + API::get()); + Registry::register_resource( + API::get()); + Registry::register_resource( + API::get()); + Registry::register_resource(API::get()); Registry::register_resource(API::get(), GenericComponent::resource_registration()); Registry::register_resource(API::get(), Motor::resource_registration()); Registry::register_resource(API::get(), MovementSensor::resource_registration()); Registry::register_resource(API::get(), PowerSensor::resource_registration()); - Registry::register_resource(API::get(), Sensor::resource_registration()); + Registry::register_resource( + API::get()); Registry::register_resource(API::get(), Servo::resource_registration()); } From 15f5070ffb8f5456e592c54236cd96ba4300bbd0 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Mon, 29 Jan 2024 16:10:49 -0500 Subject: [PATCH 04/20] everything compiling, tests passing --- src/viam/examples/camera/example_camera.cpp | 4 + src/viam/examples/dial/example_dial.cpp | 4 + .../dial_api_key/example_dial_api_key.cpp | 4 + src/viam/examples/modules/complex/client.cpp | 4 + src/viam/examples/motor/example_motor.cpp | 4 + src/viam/sdk/CMakeLists.txt | 8 +- src/viam/sdk/common/viam.cpp | 53 -------- src/viam/sdk/common/viam.hpp | 15 --- src/viam/sdk/components/base/base.cpp | 38 ------ src/viam/sdk/components/base/base.hpp | 17 --- src/viam/sdk/components/board/board.cpp | 38 ------ src/viam/sdk/components/board/board.hpp | 18 --- src/viam/sdk/components/camera/camera.cpp | 39 ------ src/viam/sdk/components/camera/camera.hpp | 19 --- src/viam/sdk/components/encoder/encoder.cpp | 39 ------ src/viam/sdk/components/encoder/encoder.hpp | 19 --- src/viam/sdk/components/generic/generic.cpp | 41 ------ src/viam/sdk/components/generic/generic.hpp | 21 --- src/viam/sdk/components/motor/client.hpp | 1 - src/viam/sdk/components/motor/motor.cpp | 37 ----- src/viam/sdk/components/motor/motor.hpp | 19 --- .../movement_sensor/movement_sensor.cpp | 16 --- .../movement_sensor/movement_sensor.hpp | 3 - .../components/power_sensor/power_sensor.cpp | 16 --- .../components/power_sensor/power_sensor.hpp | 3 - src/viam/sdk/components/sensor/sensor.cpp | 14 -- src/viam/sdk/components/sensor/sensor.hpp | 3 - src/viam/sdk/components/servo/servo.cpp | 37 ----- src/viam/sdk/components/servo/servo.hpp | 15 --- src/viam/sdk/registry/registry.cpp | 126 ++++++++++++++++-- src/viam/sdk/registry/registry.hpp | 33 ++++- .../sdk/resource/resource_server_base.hpp | 1 - src/viam/sdk/robot/client.hpp | 4 +- src/viam/sdk/rpc/server.cpp | 18 ++- src/viam/sdk/rpc/server.hpp | 3 + src/viam/sdk/services/generic/generic.cpp | 40 ------ src/viam/sdk/services/generic/generic.hpp | 21 --- src/viam/sdk/services/mlmodel/mlmodel.cpp | 40 ------ src/viam/sdk/services/mlmodel/mlmodel.hpp | 15 --- src/viam/sdk/services/motion/client.hpp | 2 + src/viam/sdk/services/motion/motion.cpp | 38 ------ src/viam/sdk/services/motion/motion.hpp | 18 --- src/viam/sdk/tests/test_robot.cpp | 8 ++ src/viam/sdk/tests/test_utils.hpp | 5 +- 44 files changed, 199 insertions(+), 722 deletions(-) diff --git a/src/viam/examples/camera/example_camera.cpp b/src/viam/examples/camera/example_camera.cpp index d32e56e72..de45452bb 100644 --- a/src/viam/examples/camera/example_camera.cpp +++ b/src/viam/examples/camera/example_camera.cpp @@ -15,6 +15,10 @@ int main() { using std::endl; namespace vs = ::viam::sdk; try { + // initialize the Registry to ensure all built-in components and gRPC server reflection + // are supported. + vs::Registry::initialize(); + // If you want to connect to a remote robot, this should be the url of the robot // Ex: xxx.xxx.viam.cloud std::string robot_address("localhost:8080"); diff --git a/src/viam/examples/dial/example_dial.cpp b/src/viam/examples/dial/example_dial.cpp index 84770ad0a..ed6b95c26 100644 --- a/src/viam/examples/dial/example_dial.cpp +++ b/src/viam/examples/dial/example_dial.cpp @@ -36,6 +36,10 @@ int main() { std::string address(uri); Options options(1, opts); + // initialize the Registry to ensure all built-in components and gRPC server reflection + // are supported. + Registry::initialize(); + // connect to robot, ensure we can refresh it std::shared_ptr robot = RobotClient::at_address(address, options); diff --git a/src/viam/examples/dial_api_key/example_dial_api_key.cpp b/src/viam/examples/dial_api_key/example_dial_api_key.cpp index d49acd117..33391e18b 100644 --- a/src/viam/examples/dial_api_key/example_dial_api_key.cpp +++ b/src/viam/examples/dial_api_key/example_dial_api_key.cpp @@ -38,6 +38,10 @@ int main() { std::string address(uri); Options options(1, opts); + // initialize the Registry to ensure all built-in components and gRPC server reflection + // are supported. + Registry::initialize(); + // connect to robot, ensure we can refresh it std::shared_ptr robot = RobotClient::at_address(address, options); diff --git a/src/viam/examples/modules/complex/client.cpp b/src/viam/examples/modules/complex/client.cpp index a24530edc..d658ceef4 100644 --- a/src/viam/examples/modules/complex/client.cpp +++ b/src/viam/examples/modules/complex/client.cpp @@ -37,6 +37,10 @@ int main() { std::string address(uri); Options options(1, opts); + // initialize the Registry to ensure all built-in components and gRPC server reflection + // are supported. + Registry::initialize(); + // Register custom gizmo and summation API so robot client can access resources // of that type from the server. Registry::register_resource(API::get(), Gizmo::resource_registration()); diff --git a/src/viam/examples/motor/example_motor.cpp b/src/viam/examples/motor/example_motor.cpp index 60b78ef2c..339c96a6d 100644 --- a/src/viam/examples/motor/example_motor.cpp +++ b/src/viam/examples/motor/example_motor.cpp @@ -27,6 +27,10 @@ int main() { namespace vs = ::viam::sdk; try { + // initialize the Registry to ensure all built-in components and gRPC server reflection + // are supported. + vs::Registry::initialize(); + // If you want to connect to a remote robot, this should be the url of the robot // Ex: xxx.xxx.viam.cloud std::string robot_address("localhost:8080"); diff --git a/src/viam/sdk/CMakeLists.txt b/src/viam/sdk/CMakeLists.txt index 5a5cda6c8..8b5990d55 100644 --- a/src/viam/sdk/CMakeLists.txt +++ b/src/viam/sdk/CMakeLists.txt @@ -36,13 +36,6 @@ endif() target_sources(viamsdk PRIVATE - # TODO(RSDK-1742): we have to put `registry` up top here because we need the - # registry types to be defined first, before anything tries to init them. - # this obviously isn't great. it breaks up stylistic ordering and is brittle - # when we need to add updates. we should refactor to make this unnecessary. - # consider making all necessary runtime values a single `context` that has to - # be initialized within main before anything else happens? - registry/registry.cpp common/client_helper.cpp common/linear_algebra.cpp common/pose.cpp @@ -87,6 +80,7 @@ target_sources(viamsdk module/service.cpp module/signal_manager.cpp referenceframe/frame.cpp + registry/registry.cpp resource/resource.cpp resource/resource_api.cpp resource/resource_manager.cpp diff --git a/src/viam/sdk/common/viam.cpp b/src/viam/sdk/common/viam.cpp index d509f1ef9..8b1378917 100644 --- a/src/viam/sdk/common/viam.cpp +++ b/src/viam/sdk/common/viam.cpp @@ -1,54 +1 @@ -#include "component/base/v1/base.grpc.pb.h" -#include "component/board/v1/board.grpc.pb.h" -#include "component/camera/v1/camera.grpc.pb.h" -#include "component/encoder/v1/encoder.grpc.pb.h" -#include "component/sensor/v1/sensor.grpc.pb.h" -#include "viam/sdk/components/base/base.hpp" -#include "viam/sdk/components/base/client.hpp" -#include "viam/sdk/components/base/server.hpp" -#include "viam/sdk/components/board/board.hpp" -#include "viam/sdk/components/board/client.hpp" -#include "viam/sdk/components/board/server.hpp" -#include "viam/sdk/components/camera/camera.hpp" -#include "viam/sdk/components/camera/client.hpp" -#include "viam/sdk/components/camera/server.hpp" -#include "viam/sdk/components/encoder/client.hpp" -#include "viam/sdk/components/encoder/encoder.hpp" -#include "viam/sdk/components/encoder/server.hpp" -#include "viam/sdk/components/motor/motor.hpp" -#include "viam/sdk/components/movement_sensor/movement_sensor.hpp" -#include "viam/sdk/components/power_sensor/power_sensor.hpp" -#include "viam/sdk/components/sensor/client.hpp" -#include "viam/sdk/components/sensor/sensor.hpp" -#include "viam/sdk/components/sensor/server.hpp" -#include "viam/sdk/components/servo/servo.hpp" -#include "viam/sdk/registry/registry.hpp" -#include -#include -namespace viam { -namespace sdk { - -ViamInit::ViamInit() { - Registry::register_resource( - API::get()); - Registry::register_resource( - API::get()); - Registry::register_resource( - API::get()); - Registry::register_resource(API::get()); - Registry::register_resource(API::get(), - GenericComponent::resource_registration()); - Registry::register_resource(API::get(), Motor::resource_registration()); - Registry::register_resource(API::get(), - MovementSensor::resource_registration()); - Registry::register_resource(API::get(), PowerSensor::resource_registration()); - Registry::register_resource( - API::get()); - Registry::register_resource(API::get(), Servo::resource_registration()); -} - -} // namespace sdk -} // namespace viam diff --git a/src/viam/sdk/common/viam.hpp b/src/viam/sdk/common/viam.hpp index 2418f85a4..8b1378917 100644 --- a/src/viam/sdk/common/viam.hpp +++ b/src/viam/sdk/common/viam.hpp @@ -1,16 +1 @@ -#pragma once -namespace viam { -namespace sdk { - -class ViamInit { - public: - ViamInit(); - ~ViamInit(); - - private: - static bool initialized_; -}; - -} // namespace sdk -} // namespace viam diff --git a/src/viam/sdk/components/base/base.cpp b/src/viam/sdk/components/base/base.cpp index 9a6630168..d2740fdb9 100644 --- a/src/viam/sdk/components/base/base.cpp +++ b/src/viam/sdk/components/base/base.cpp @@ -6,39 +6,11 @@ #include #include -#include -#include -#include #include namespace viam { namespace sdk { -BaseRegistration::BaseRegistration(const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor){}; - -std::shared_ptr BaseRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto bs = std::make_shared(manager); - server.register_service(bs.get()); - return bs; -}; - -std::shared_ptr BaseRegistration::create_rpc_client(std::string name, - std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -}; - -std::shared_ptr Base::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(viam::component::base::v1::BaseService::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor for the base service"); - } - return std::make_shared(sd); -} - API Base::api() const { return API::get(); } @@ -68,15 +40,5 @@ bool operator==(const Base::properties& lhs, const Base::properties& rhs) { Base::Base(std::string name) : Component(std::move(name)){}; -namespace { -bool init() { - Registry::register_resource(API::get(), Base::resource_registration()); - return true; -}; - -// NOLINTNEXTLINE -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/base/base.hpp b/src/viam/sdk/components/base/base.hpp index e4120b017..168802d99 100644 --- a/src/viam/sdk/components/base/base.hpp +++ b/src/viam/sdk/components/base/base.hpp @@ -11,8 +11,6 @@ #include #include #include -#include -#include #include namespace viam { @@ -20,18 +18,6 @@ namespace sdk { /// @defgroup Base Classes related to the Base component. -/// @class BaseRegistration -/// @brief Defines a `ResourceRegistration` for the `Base` component. -/// @ingroup Base -class BaseRegistration : public ResourceRegistration { - public: - explicit BaseRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; - /// @class Base base.hpp "components/base/base.hpp" /// @brief A `Base` is the platform that the other parts of a mobile robot attach to. /// @ingroup Base @@ -52,9 +38,6 @@ class Base : public Component, public Stoppable { friend std::ostream& operator<<(std::ostream& os, const properties& v); friend bool operator==(const properties& lhs, const properties& rhs); - // functions shared across all components - static std::shared_ptr resource_registration(); - /// @brief Move a robot's base in a straight line by a given distance. This method blocks /// until completed or cancelled /// @param distance_mm Desired travel distance in millimeters diff --git a/src/viam/sdk/components/board/board.cpp b/src/viam/sdk/components/board/board.cpp index 56e69456a..e15d72c2e 100644 --- a/src/viam/sdk/components/board/board.cpp +++ b/src/viam/sdk/components/board/board.cpp @@ -6,39 +6,11 @@ #include #include -#include -#include -#include #include namespace viam { namespace sdk { -BoardRegistration::BoardRegistration(const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor) {} - -std::shared_ptr BoardRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto bs = std::make_shared(manager); - server.register_service(bs.get()); - return bs; -} - -std::shared_ptr BoardRegistration::create_rpc_client( - std::string name, std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -} - -std::shared_ptr Board::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(viam::component::board::v1::BoardService::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor for the board service"); - } - return std::make_shared(sd); -} - API Board::api() const { return API::get(); } @@ -153,15 +125,5 @@ bool operator==(const Board::status& lhs, const Board::status& rhs) { lhs.digital_interrupt_values == rhs.digital_interrupt_values); } -namespace { -bool init() { - Registry::register_resource(API::get(), Board::resource_registration()); - return true; -}; - -// NOLINTNEXTLINE(cert-err58-cpp) -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/board/board.hpp b/src/viam/sdk/components/board/board.hpp index d5af2a830..aeea261d9 100644 --- a/src/viam/sdk/components/board/board.hpp +++ b/src/viam/sdk/components/board/board.hpp @@ -11,26 +11,10 @@ #include #include #include -#include -#include namespace viam { namespace sdk { -/// @defgroup Board Classes related to the Board component. - -/// @class BoardRegistration -/// @brief Defines a `ResourceRegistration` for the `Board` component. -/// @ingroup Board -class BoardRegistration : public ResourceRegistration { - public: - explicit BoardRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; - /// @class Board board.hpp "components/board/board.hpp" /// @brief Represents a physical board with gpio pins, digital interrupts, and analog voltage /// reading @@ -65,8 +49,6 @@ class Board : public Component { /// The effect of these power modes depends on your physical board enum class power_mode : uint8_t { normal = 0, offline_deep = 1 }; - // functions shared across all components - static std::shared_ptr resource_registration(); API api() const override; /// @brief Creates a `status` struct from its proto representation. diff --git a/src/viam/sdk/components/camera/camera.cpp b/src/viam/sdk/components/camera/camera.cpp index ca41b9e09..b4bc4ac57 100644 --- a/src/viam/sdk/components/camera/camera.cpp +++ b/src/viam/sdk/components/camera/camera.cpp @@ -8,40 +8,11 @@ #include #include -#include -#include -#include #include namespace viam { namespace sdk { -CameraRegistration::CameraRegistration( - const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor){}; - -std::shared_ptr CameraRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto cs = std::make_shared(manager); - server.register_service(cs.get()); - return cs; -}; - -std::shared_ptr CameraRegistration::create_rpc_client( - std::string name, std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -}; - -std::shared_ptr Camera::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(viam::component::camera::v1::CameraService::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor for the camera service"); - } - return std::make_shared(sd); -} - // NOLINTNEXTLINE const std::string Camera::lazy_suffix = "+lazy"; @@ -223,15 +194,5 @@ bool operator==(const Camera::properties& lhs, const Camera::properties& rhs) { lhs.distortion_parameters == rhs.distortion_parameters; } -namespace { -bool init() { - Registry::register_resource(API::get(), Camera::resource_registration()); - return true; -}; - -// NOLINTNEXTLINE -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/camera/camera.hpp b/src/viam/sdk/components/camera/camera.hpp index da9f46428..6dd4eb5e3 100644 --- a/src/viam/sdk/components/camera/camera.hpp +++ b/src/viam/sdk/components/camera/camera.hpp @@ -14,27 +14,11 @@ #include #include #include -#include #include -#include namespace viam { namespace sdk { -/// @defgroup Camera Classes related to the Camera component. - -/// @class CameraRegistration -/// @brief Defines a `ResourceRegistration` for the `Camera` component. -/// @ingroup Camera -class CameraRegistration : public ResourceRegistration { - public: - explicit CameraRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; - /// @class Camera camera.hpp "components/camera/camera.hpp" /// @brief A `Camera` represents any physical hardware that can capture frames. /// @ingroup Camera @@ -99,9 +83,6 @@ class Camera : public Component { response_metadata metadata; }; - /// @brief Creates a `ResourceRegistration` for the `Camera` component. - static std::shared_ptr resource_registration(); - /// @brief remove any extra suffix's from the mime type string. static std::string normalize_mime_type(const std::string& str); diff --git a/src/viam/sdk/components/encoder/encoder.cpp b/src/viam/sdk/components/encoder/encoder.cpp index cca647768..52a2505c7 100644 --- a/src/viam/sdk/components/encoder/encoder.cpp +++ b/src/viam/sdk/components/encoder/encoder.cpp @@ -6,40 +6,11 @@ #include #include -#include -#include -#include #include namespace viam { namespace sdk { -EncoderRegistration::EncoderRegistration( - const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor){}; - -std::shared_ptr EncoderRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto es = std::make_shared(manager); - server.register_service(es.get()); - return es; -}; - -std::shared_ptr EncoderRegistration::create_rpc_client( - std::string name, std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -}; - -std::shared_ptr Encoder::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(viam::component::encoder::v1::EncoderService::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor for the encoder service"); - } - return std::make_shared(sd); -} - API Encoder::api() const { return API::get(); } @@ -125,15 +96,5 @@ bool operator==(const Encoder::properties& lhs, const Encoder::properties& rhs) lhs.angle_degrees_supported == rhs.angle_degrees_supported); } -namespace { -bool init() { - Registry::register_resource(API::get(), Encoder::resource_registration()); - return true; -}; - -// NOLINTNEXTLINE -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/encoder/encoder.hpp b/src/viam/sdk/components/encoder/encoder.hpp index 66eed0ffe..85ed6c450 100644 --- a/src/viam/sdk/components/encoder/encoder.hpp +++ b/src/viam/sdk/components/encoder/encoder.hpp @@ -10,26 +10,10 @@ #include #include #include -#include -#include namespace viam { namespace sdk { -/// @defgroup Encoder Classes related to the Encoder component. - -/// @class EncoderRegistration -/// @brief Defines a `ResourceRegistration` for the `Encoder` component. -/// @ingroup Encoder -class EncoderRegistration : public ResourceRegistration { - public: - explicit EncoderRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; - /// @class Encoder encoder.hpp "components/encoder/encoder.hpp" /// @brief An encoder is a device that is hooked up to motors to report a position /// @ingroup Encoder @@ -62,9 +46,6 @@ class Encoder : public Component { bool angle_degrees_supported; }; - // functions shared across all components - static std::shared_ptr resource_registration(); - /// @brief Creates a `position_type` struct from its proto representation. static position_type from_proto(viam::component::encoder::v1::PositionType proto); diff --git a/src/viam/sdk/components/generic/generic.cpp b/src/viam/sdk/components/generic/generic.cpp index 4e8ebefe4..1e9929d6a 100644 --- a/src/viam/sdk/components/generic/generic.cpp +++ b/src/viam/sdk/components/generic/generic.cpp @@ -7,41 +7,11 @@ #include #include -#include -#include -#include #include namespace viam { namespace sdk { -GenericComponentRegistration::GenericComponentRegistration( - const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor){}; - -std::shared_ptr GenericComponentRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto gs = std::make_shared(std::move(manager)); - server.register_service(gs.get()); - return gs; -}; - -std::shared_ptr GenericComponentRegistration::create_rpc_client( - std::string name, std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -}; - -std::shared_ptr GenericComponent::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(viam::component::generic::v1::GenericService::service_full_name()); - if (!sd) { - throw std::runtime_error( - "Unable to get service descriptor for the generic component service"); - } - return std::make_shared(sd); -} - API GenericComponent::api() const { return API::get(); } @@ -52,16 +22,5 @@ API API::traits::api() { GenericComponent::GenericComponent(std::string name) : Component(std::move(name)){}; -namespace { -bool init() { - Registry::register_resource(API::get(), - GenericComponent::resource_registration()); - return true; -}; - -// NOLINTNEXTLINE -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/generic/generic.hpp b/src/viam/sdk/components/generic/generic.hpp index 6e0a08fb7..da471227c 100644 --- a/src/viam/sdk/components/generic/generic.hpp +++ b/src/viam/sdk/components/generic/generic.hpp @@ -9,28 +9,10 @@ #include #include #include -#include -#include namespace viam { namespace sdk { -// TODO(RSDK-3030): one class per header -/// @defgroup GenericComponent Classes related to the generic component. - -/// @class GenericComponentRegistration -/// @brief Defines a `ResourceRegistration` for the generic component. -/// @ingroup GenericComponent -class GenericComponentRegistration : public ResourceRegistration { - public: - explicit GenericComponentRegistration( - const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; - /// @class GenericComponent generic.hpp "components/generic/generic.hpp" /// @brief A `GenericComponent` represents any component that can execute arbitrary commands. /// @ingroup GenericComponent @@ -39,9 +21,6 @@ class GenericComponentRegistration : public ResourceRegistration { /// specific generic implementations. This class cannot be used on its own. class GenericComponent : public Component { public: - /// @brief Creates a `ResourceRegistration` for `GenericComponent`. - static std::shared_ptr resource_registration(); - /// @brief Send/receive arbitrary commands to the resource. /// @param command the command to execute. /// @return The result of the executed command. diff --git a/src/viam/sdk/components/motor/client.hpp b/src/viam/sdk/components/motor/client.hpp index 54be57cc6..147535e80 100644 --- a/src/viam/sdk/components/motor/client.hpp +++ b/src/viam/sdk/components/motor/client.hpp @@ -10,7 +10,6 @@ #include #include #include -#include namespace viam { namespace sdk { diff --git a/src/viam/sdk/components/motor/motor.cpp b/src/viam/sdk/components/motor/motor.cpp index 6b79b57e9..7e87cc25b 100644 --- a/src/viam/sdk/components/motor/motor.cpp +++ b/src/viam/sdk/components/motor/motor.cpp @@ -6,38 +6,11 @@ #include #include -#include -#include -#include #include namespace viam { namespace sdk { -std::shared_ptr MotorRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto ms = std::make_shared(std::move(manager)); - server.register_service(ms.get()); - return ms; -}; - -std::shared_ptr MotorRegistration::create_rpc_client( - std::string name, std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -}; -MotorRegistration::MotorRegistration(const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor){}; - -std::shared_ptr Motor::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(viam::component::motor::v1::MotorService::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor for the motor service"); - } - return std::make_shared(sd); -} - Motor::position Motor::from_proto(viam::component::motor::v1::GetPositionResponse proto) { return proto.position(); } @@ -93,15 +66,5 @@ bool operator==(const Motor::properties& lhs, const Motor::properties& rhs) { return (lhs.position_reporting == rhs.position_reporting); } -namespace { -bool init() { - Registry::register_resource(API::get(), Motor::resource_registration()); - return true; -}; - -// NOLINTNEXTLINE -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/motor/motor.hpp b/src/viam/sdk/components/motor/motor.hpp index 65d9b64d4..173cd69e3 100644 --- a/src/viam/sdk/components/motor/motor.hpp +++ b/src/viam/sdk/components/motor/motor.hpp @@ -10,27 +10,11 @@ #include #include #include -#include -#include #include namespace viam { namespace sdk { -/// @defgroup Motor Classes related to the Motor component. - -/// @class MotorRegistration -/// @brief Defines a `ResourceRegistration` for the `Motor` component. -/// @ingroup Motor -class MotorRegistration : public ResourceRegistration { - public: - explicit MotorRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; - /// @class Motor motor.hpp "components/motor/motor.hpp" /// @brief A `Motor` represents physical hardware that controls the rotation of an axle /// @ingroup Motor @@ -61,9 +45,6 @@ class Motor : public Component, public Stoppable { bool position_reporting; }; - // functions shared across all components - static std::shared_ptr resource_registration(); - /// @brief Creates a `position` struct from its proto representation. static position from_proto(viam::component::motor::v1::GetPositionResponse proto); diff --git a/src/viam/sdk/components/movement_sensor/movement_sensor.cpp b/src/viam/sdk/components/movement_sensor/movement_sensor.cpp index e93e7c794..f8e5655a1 100644 --- a/src/viam/sdk/components/movement_sensor/movement_sensor.cpp +++ b/src/viam/sdk/components/movement_sensor/movement_sensor.cpp @@ -6,9 +6,6 @@ #include #include -#include -#include -#include #include namespace viam { @@ -104,18 +101,5 @@ bool operator==(const MovementSensor::properties& lhs, const MovementSensor::pro lhs.linear_acceleration_supported == rhs.linear_acceleration_supported); } -namespace { -bool init() { - Registry::register_resource( - API::get()); - return true; -}; - -// NOLINTNEXTLINE(cert-err58-cpp) -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/movement_sensor/movement_sensor.hpp b/src/viam/sdk/components/movement_sensor/movement_sensor.hpp index 240e334e1..bd22c2ab1 100644 --- a/src/viam/sdk/components/movement_sensor/movement_sensor.hpp +++ b/src/viam/sdk/components/movement_sensor/movement_sensor.hpp @@ -10,10 +10,7 @@ #include #include #include -#include #include -#include -#include #include namespace viam { diff --git a/src/viam/sdk/components/power_sensor/power_sensor.cpp b/src/viam/sdk/components/power_sensor/power_sensor.cpp index 70eacff35..137c9a7e2 100644 --- a/src/viam/sdk/components/power_sensor/power_sensor.cpp +++ b/src/viam/sdk/components/power_sensor/power_sensor.cpp @@ -4,9 +4,6 @@ #include #include -#include -#include -#include #include using namespace viam::component::powersensor::v1; @@ -58,18 +55,5 @@ bool operator==(const PowerSensor::current& lhs, const PowerSensor::current& rhs return (lhs.amperes == rhs.amperes && lhs.is_ac == rhs.is_ac); } -namespace { -bool init() { - Registry::register_resource( - API::get()); - return true; -}; - -// NOLINTNEXTLINE(cert-err58-cpp) -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/power_sensor/power_sensor.hpp b/src/viam/sdk/components/power_sensor/power_sensor.hpp index 402107fec..b2052b7af 100644 --- a/src/viam/sdk/components/power_sensor/power_sensor.hpp +++ b/src/viam/sdk/components/power_sensor/power_sensor.hpp @@ -9,10 +9,7 @@ #include #include -#include #include -#include -#include using namespace viam::component::powersensor::v1; diff --git a/src/viam/sdk/components/sensor/sensor.cpp b/src/viam/sdk/components/sensor/sensor.cpp index 717c09c81..ad171d2b6 100644 --- a/src/viam/sdk/components/sensor/sensor.cpp +++ b/src/viam/sdk/components/sensor/sensor.cpp @@ -3,9 +3,6 @@ #include #include -#include -#include -#include #include namespace viam { @@ -19,16 +16,5 @@ API API::traits::api() { return {kRDK, kComponent, "sensor"}; } -namespace { -bool init() { - Registry::register_resource( - API::get()); - return true; -}; - -// NOLINTNEXTLINE(cert-err58-cpp) -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/sensor/sensor.hpp b/src/viam/sdk/components/sensor/sensor.hpp index d9a6d914b..a3af337f6 100644 --- a/src/viam/sdk/components/sensor/sensor.hpp +++ b/src/viam/sdk/components/sensor/sensor.hpp @@ -9,10 +9,7 @@ #include #include -#include #include -#include -#include namespace viam { namespace sdk { diff --git a/src/viam/sdk/components/servo/servo.cpp b/src/viam/sdk/components/servo/servo.cpp index 07b779501..2b6b558b9 100644 --- a/src/viam/sdk/components/servo/servo.cpp +++ b/src/viam/sdk/components/servo/servo.cpp @@ -4,38 +4,11 @@ #include #include -#include -#include -#include #include namespace viam { namespace sdk { -std::shared_ptr ServoRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto ss = std::make_shared(std::move(manager)); - server.register_service(ss.get()); - return ss; -}; - -std::shared_ptr ServoRegistration::create_rpc_client( - std::string name, std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -}; -ServoRegistration::ServoRegistration(const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor){}; - -std::shared_ptr Servo::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(viam::component::servo::v1::ServoService::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor for the servo service"); - } - return std::make_shared(sd); -} - API Servo::api() const { return API::get(); } @@ -48,15 +21,5 @@ Servo::position Servo::from_proto(viam::component::servo::v1::GetPositionRespons return proto.position_deg(); } -namespace { -bool init() { - Registry::register_resource(API::get(), Servo::resource_registration()); - return true; -}; - -// NOLINTNEXTLINE -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/servo/servo.hpp b/src/viam/sdk/components/servo/servo.hpp index f94a7a421..d9012b66f 100644 --- a/src/viam/sdk/components/servo/servo.hpp +++ b/src/viam/sdk/components/servo/servo.hpp @@ -8,8 +8,6 @@ #include #include #include -#include -#include #include namespace viam { @@ -17,18 +15,6 @@ namespace sdk { /// @defgroup Servo Classes related to the Servo component. -/// @class ServoRegistration -/// @brief Defines a `ResourceRegistration` for the `Servo` component. -/// @ingroup Servo -class ServoRegistration : public ResourceRegistration { - public: - explicit ServoRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; - /// @class Servo servo.hpp "components/servo/servo.hpp" /// @ingroup Servo /// @@ -40,7 +26,6 @@ class Servo : public Component, public Stoppable { /// @brief Current position of the servo relative to its home typedef uint32_t position; - static std::shared_ptr resource_registration(); API api() const override; /// @brief Creates a `position` struct from its proto representation. diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 37492497e..4d35cd719 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -9,13 +9,64 @@ #include #include #include - +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include - +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include #include +#include +#include +#include +#include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include #include +#include +#include +#include +#include +#include +#include +#include +#include #include namespace viam { @@ -82,12 +133,14 @@ const google::protobuf::ServiceDescriptor* Registry::get_service_descriptor_( return sd; } -std::unordered_map> Registry::registered_models() { - std::unordered_map> registry; - for (auto& resource : resources_) { - registry.emplace(resource.first, resource.second); - } - return registry; +const std::unordered_map>& +Registry::registered_resources() { + return apis_; +} + +const std::unordered_map>& +Registry::registered_models() { + return resources_; } // NOLINTNEXTLINE(readability-convert-member-functions-to-static) @@ -102,8 +155,65 @@ const google::protobuf::ServiceDescriptor* ResourceRegistration::service_descrip return service_descriptor_; } +void register_resources() { + // Register all components + Registry::register_resource( + API::get()); + Registry::register_resource( + API::get()); + Registry::register_resource( + API::get()); + Registry::register_resource(API::get()); + Registry::register_resource( + API::get()); + Registry::register_resource( + API::get()); + Registry::register_resource( + API::get()); + Registry::register_resource( + API::get()); + Registry::register_resource( + API::get()); + Registry::register_resource( + API::get()); + + // Register all services + Registry::register_resource(API::get()); + Registry::register_resource(API::get()); + Registry::register_resource( + API::get()); +} + +bool Registry::is_initialized() { + return initialized_; +} + +void Registry::initialize() { + if (initialized_) { + BOOST_LOG_TRIVIAL(warning) + << "Attempted to initialize the Registry but it was already initialized."; + return; + } + initialized_ = true; + register_resources(); + grpc::reflection::InitProtoReflectionServerBuilderPlugin(); +} + std::unordered_map> Registry::resources_; std::unordered_map> Registry::apis_; +bool Registry::initialized_{false}; } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index cc32ba943..96fd32aee 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -18,6 +18,7 @@ #include #include #include +#include namespace viam { namespace sdk { @@ -101,7 +102,7 @@ class ModelRegistration { // is provided in construction. No dependencies are returned. Model model_; API api_; - static const std::vector default_validator(ResourceConfig cfg) { + static std::vector default_validator(ResourceConfig cfg) { return {}; }; }; @@ -113,19 +114,25 @@ class Registry { /// @brief Registers a resource with the Registry. /// @param resource An object containing resource registration information. /// @throws `std::runtime_error` if the resource has already been registered. + /// @throws `std::runtime_error` if `initialize` has not been called. static void register_model(std::shared_ptr resource); /// @brief Lookup a given registered resource. /// @param name The name of the resource to lookup. /// @return a `shared_ptr` to the resource's registration data. + /// @throws `std::runtime_error` if `initialize` has not been called. static std::shared_ptr lookup_model(std::string name); /// @brief Lookup a given registered resource. /// @param api The api of the resource to lookup. /// @param model The model of the resource to lookup. /// @return a `shared_ptr` to the resource's registration data. + /// @throws `std::runtime_error` if `initialize` has not been called. static std::shared_ptr lookup_model(API api, Model model); + /// @brief Register an api. + /// @param api The api to be registered. + /// @throws `std::runtime_error` if `initialize` has not been called. template static void register_resource(API api) { class ResourceRegistration2 final : public ResourceRegistration { @@ -152,19 +159,37 @@ class Registry { /// @brief Register an api. /// @param api The api to be registered. /// @param resource_registration `ResourceRegistration` with resource functionality. + /// @throws `std::runtime_error` if `initialize` has not been called. static void register_resource(API api, std::shared_ptr resource_registration); /// @brief Lookup a registered api. /// @param api The api to lookup. /// @return A `shared_ptr` to the registered api's `ResourceRegistration`. + /// @throws `std::runtime_error` if `initialize` has not been called. static std::shared_ptr lookup_resource(API api); - /// @brief Provide information on registered resources. - /// @return A map from name to `ModelRegistration` of all registered resources. - static std::unordered_map> registered_models(); + /// @brief Provide information on registered resource models. + /// @return A map from name to `ModelRegistration` of all registered resource models. + /// @throws `std::runtime_error` if `initialize` has not been called. + static const std::unordered_map>& + registered_models(); + + /// @brief Provide access to registered resources. + /// @return A map from `API` to `ResourceRegistration` of all registered resources. + /// @throws `std::runtime_error` if `initialize` has not been called. + static const std::unordered_map>& + registered_resources(); + + /// @brief Initialized the Viam registry. No-op if it has already been called. + static void initialize(); + + /// @brief Returns whether or not the Registry has been `initialize`d. + static bool is_initialized(); private: + static bool initialized_; + static const google::protobuf::ServiceDescriptor* get_service_descriptor_( const char* service_full_name); static std::unordered_map> resources_; diff --git a/src/viam/sdk/resource/resource_server_base.hpp b/src/viam/sdk/resource/resource_server_base.hpp index 44e73ff81..a7a954090 100644 --- a/src/viam/sdk/resource/resource_server_base.hpp +++ b/src/viam/sdk/resource/resource_server_base.hpp @@ -1,7 +1,6 @@ #pragma once #include -#include namespace viam { namespace sdk { diff --git a/src/viam/sdk/robot/client.hpp b/src/viam/sdk/robot/client.hpp index cf2a3c84c..2597f4b01 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -1,4 +1,4 @@ -/// @file robot/client.cpp +/// @file robot/client.hpp /// /// @brief gRPC client implementation for a `robot`. #pragma once @@ -32,7 +32,7 @@ using viam::robot::v1::Status; // TODO(RSDK-1742) replace all `ResourceName` references in API with `Name` /// @defgroup Robot Classes related to a Robot representation. -/// @class RobotClient client.h "robot/client.h" +/// @class RobotClient client.hpp "robot/client.hpp" /// @brief gRPC client for a robot, to be used for all interactions with a robot. /// There are two ways to instantiate a robot: /// - `RobotClient::at_address(...)` diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index bca7c1711..30b28bf0f 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -1,12 +1,23 @@ #include -#include #include +#include + namespace viam { namespace sdk { -Server::Server() : builder_(std::make_unique()) {} +Server::Server() : builder_(std::make_unique()) { + auto new_manager = std::make_shared(); + // CR erodkin: is this the best place to do this? Probably it makes more sense when we actually + // register the service. see if we can figure out how to do that. Related: if we can do it right + // then we don't need to manage resources separately in the module manager probably? it'll also + // have a server and that server can track everything for them. But we'll need to test that! + for (const auto& rr : Registry::registered_resources()) { + auto server = rr.second->create_resource_server(new_manager, *this); + managed_servers_.push_back(server); + } +} Server::~Server() { shutdown(); @@ -25,9 +36,6 @@ void Server::start() { throw std::runtime_error("Attempted to start server that was already running"); } - // CR erodkin: this is supposed to be called at static initialization. We can get close - // enough (I suspect) by putting this into the `TheOneAndOnly` call. - grpc::reflection::InitProtoReflectionServerBuilderPlugin(); server_ = builder_->BuildAndStart(); builder_ = nullptr; } diff --git a/src/viam/sdk/rpc/server.hpp b/src/viam/sdk/rpc/server.hpp index 28c32c281..7d8b18fa1 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -7,6 +7,8 @@ #include #include +#include + namespace viam { // needed for later `friend` declaration. @@ -52,6 +54,7 @@ class Server { friend class ::viam::sdktests::TestServer; private: + std::vector> managed_servers_; std::unique_ptr builder_; std::unique_ptr server_; }; diff --git a/src/viam/sdk/services/generic/generic.cpp b/src/viam/sdk/services/generic/generic.cpp index 2423ee84e..4fe7f446a 100644 --- a/src/viam/sdk/services/generic/generic.cpp +++ b/src/viam/sdk/services/generic/generic.cpp @@ -7,40 +7,11 @@ #include #include -#include #include -#include -#include namespace viam { namespace sdk { -GenericServiceRegistration::GenericServiceRegistration( - const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor){}; - -std::shared_ptr GenericServiceRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto gs = std::make_shared(std::move(manager)); - server.register_service(gs.get()); - return gs; -}; - -std::shared_ptr GenericServiceRegistration::create_rpc_client( - std::string name, std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -}; - -std::shared_ptr GenericService::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(viam::component::generic::v1::GenericService::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor for the generic service"); - } - return std::make_shared(sd); -} - API GenericService::api() const { return API::get(); } @@ -51,16 +22,5 @@ API API::traits::api() { GenericService::GenericService(std::string name) : Service(std::move(name)){}; -namespace { -bool init() { - Registry::register_resource(API::get(), - GenericService::resource_registration()); - return true; -}; - -// NOLINTNEXTLINE -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/services/generic/generic.hpp b/src/viam/sdk/services/generic/generic.hpp index f6ab0218e..4d0568038 100644 --- a/src/viam/sdk/services/generic/generic.hpp +++ b/src/viam/sdk/services/generic/generic.hpp @@ -9,29 +9,11 @@ #include #include #include -#include -#include #include namespace viam { namespace sdk { -// TODO(RSDK-3030): one class per header -/// @defgroup GenericService Classes related to the generic service. - -/// @class GenericServiceRegistration -/// @brief Defines a `ResourceRegistration` for the `GenericService`. -/// @ingroup GenericService -class GenericServiceRegistration : public ResourceRegistration { - public: - explicit GenericServiceRegistration( - const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; - /// @class GenericService generic.hpp "services/generic/generic.hpp" /// @brief A `GenericService` represents any service that can execute arbitrary commands. /// @ingroup GenericService @@ -40,9 +22,6 @@ class GenericServiceRegistration : public ResourceRegistration { /// specific generic service implementations. This class cannot be used on its own. class GenericService : public Service { public: - /// @brief Creates a `ResourceRegistration` for the `GenericService`. - static std::shared_ptr resource_registration(); - /// @brief Send/receive arbitrary commands to the resource. /// @param command the command to execute. /// @return The result of the executed command. diff --git a/src/viam/sdk/services/mlmodel/mlmodel.cpp b/src/viam/sdk/services/mlmodel/mlmodel.cpp index 8cad6272d..bee188919 100644 --- a/src/viam/sdk/services/mlmodel/mlmodel.cpp +++ b/src/viam/sdk/services/mlmodel/mlmodel.cpp @@ -15,39 +15,10 @@ #include #include -#include -#include namespace viam { namespace sdk { -MLModelServiceRegistration::MLModelServiceRegistration( - const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor) {} - -std::shared_ptr MLModelServiceRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto mlms = std::make_shared(std::move(manager)); - server.register_service(mlms.get()); - return mlms; -}; - -std::shared_ptr MLModelServiceRegistration::create_rpc_client( - std::string name, std::shared_ptr channel) { - return std::make_shared(std::move(name), std::move(channel)); -}; - -std::shared_ptr MLModelService::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(viam::service::mlmodel::v1::MLModelService::service_full_name()); - if (!sd) { - // TODO: Throw viam exception once PR #100 merges. - throw std::runtime_error("Unable to get service descriptor for the camera service"); - } - return std::make_shared(sd); -} - API MLModelService::api() const { return API::get(); } @@ -178,16 +149,5 @@ MLModelService::tensor_info::data_types MLModelService::tensor_info::tensor_view MLModelService::MLModelService(std::string name) : Service(std::move(name)) {} -namespace { -bool init() { - Registry::register_resource(API::get(), - MLModelService::resource_registration()); - return true; -}; - -// NOLINTNEXTLINE -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/services/mlmodel/mlmodel.hpp b/src/viam/sdk/services/mlmodel/mlmodel.hpp index e3976845f..6d24eda1b 100644 --- a/src/viam/sdk/services/mlmodel/mlmodel.hpp +++ b/src/viam/sdk/services/mlmodel/mlmodel.hpp @@ -23,25 +23,11 @@ #include #include -#include -#include #include namespace viam { namespace sdk { -class MLModelServiceRegistration : public ResourceRegistration { - public: - explicit MLModelServiceRegistration( - const google::protobuf::ServiceDescriptor* service_descriptor); - - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr channel) override; -}; - /// @class MLModelService mlmodel.hpp "services/mlmodel/mlmodel.hpp" /// @brief Represents a machine trained learning model instance /// @@ -63,7 +49,6 @@ class MLModelService : public Service { }; public: - static std::shared_ptr resource_registration(); API api() const override; template diff --git a/src/viam/sdk/services/motion/client.hpp b/src/viam/sdk/services/motion/client.hpp index 631e2762d..19a0dddf2 100644 --- a/src/viam/sdk/services/motion/client.hpp +++ b/src/viam/sdk/services/motion/client.hpp @@ -3,6 +3,8 @@ /// @brief Implements a gRPC client for the `Motion` service. #pragma once +#include + #include #include diff --git a/src/viam/sdk/services/motion/motion.cpp b/src/viam/sdk/services/motion/motion.cpp index 51d52e354..e31a1ab4a 100644 --- a/src/viam/sdk/services/motion/motion.cpp +++ b/src/viam/sdk/services/motion/motion.cpp @@ -7,28 +7,10 @@ #include #include -#include -#include namespace viam { namespace sdk { -MotionRegistration::MotionRegistration( - const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor){}; - -std::shared_ptr MotionRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto ms = std::make_shared(manager); - server.register_service(ms.get()); - return ms; -}; - -std::shared_ptr MotionRegistration::create_rpc_client( - std::string name, std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -}; - Motion::Motion(std::string name) : Service(std::move(name)){}; service::motion::v1::Constraints Motion::constraints::to_proto() const { @@ -106,16 +88,6 @@ API API::traits::api() { return {kRDK, kService, "motion"}; } -std::shared_ptr Motion::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(service::motion::v1::MotionService::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor for the motion service"); - } - return std::make_shared(sd); -} - service::motion::v1::ObstacleDetector obstacle_detector::to_proto() const { service::motion::v1::ObstacleDetector proto; *proto.mutable_vision_service() = vision_service.to_proto(); @@ -435,15 +407,5 @@ service::motion::v1::PlanStatusWithID Motion::plan_status_with_id::to_proto() co return proto; } -namespace { -bool init() { - Registry::register_resource(API::get(), Motion::resource_registration()); - return true; -} - -// NOLINTNEXTLINE -const bool inited = init(); -} // namespace - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/services/motion/motion.hpp b/src/viam/sdk/services/motion/motion.hpp index cd37d76bf..9dd352f5f 100644 --- a/src/viam/sdk/services/motion/motion.hpp +++ b/src/viam/sdk/services/motion/motion.hpp @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -19,20 +18,6 @@ namespace viam { namespace sdk { -/// @defgroup Motion Classes related to the Motion service. - -/// @class MotionRegistration -/// @brief Defines a `ResourceRegistration` for the `Motion` service. -/// @ingroup Motion -class MotionRegistration : public ResourceRegistration { - public: - explicit MotionRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; - /// @struct obstacle_detector_name /// @brief Defines configuration for obstacle detectors by pairing a vision service name and a /// camera name. @@ -242,9 +227,6 @@ class Motion : public Service { API api() const override; - /// @brief Creates a `ResourceRegistration` for the `Motion` service. - static std::shared_ptr resource_registration(); - /// @brief Moves any compononent on the robot to a specified destination. /// @param destination Where to move the component to. /// @param name Name of the component to be moved. diff --git a/src/viam/sdk/tests/test_robot.cpp b/src/viam/sdk/tests/test_robot.cpp index 836a91cbc..486c52141 100644 --- a/src/viam/sdk/tests/test_robot.cpp +++ b/src/viam/sdk/tests/test_robot.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -37,6 +38,9 @@ BOOST_AUTO_TEST_SUITE(test_robot) // the robot client and the mock robot service. template void robot_client_to_mocks_pipeline(F&& test_case) { + if (!Registry::is_initialized()) { + Registry::initialize(); + } // Create a ResourceManager. Add a few mock resources to the // ResourceManager. Create a Server. Create a MockRobotService from the // ResourceManager and Server. Start the Server. @@ -45,8 +49,11 @@ void robot_client_to_mocks_pipeline(F&& test_case) { rm->add(std::string("mock_motor"), motor::MockMotor::get_mock_motor()); rm->add(std::string("mock_camera"), camera::MockCamera::get_mock_camera()); auto server = std::make_shared(); + std::cout << "gonna create a robot service\n" << std::flush; MockRobotService service(rm, *server); + std::cout << "gonna start a robot service\n" << std::flush; server->start(); + std::cout << " started a robot service\n" << std::flush; // Create a RobotClient to the MockRobotService over an established // in-process gRPC channel. @@ -219,6 +226,7 @@ BOOST_AUTO_TEST_CASE(test_get_resource) { [](std::shared_ptr client, MockRobotService& service) -> void { auto mock_motor = client->resource_by_name("mock_motor"); BOOST_CHECK(mock_motor); + // BOOST_CHECK(!mock_motor->is_moving()); }); } diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index f605ab704..8fc205412 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -1,7 +1,5 @@ #pragma once -#include - #include #include @@ -51,6 +49,9 @@ class TestServer { // The passed in test_case function will have access to the created ResourceClient. template void client_to_mock_pipeline(std::shared_ptr mock, F&& test_case) { + if (!Registry::is_initialized()) { + Registry::initialize(); + } // Create a ResourceManager. Add the mock resource to the ResourceManager. // Create a Server. Use the mock's API to create a resource-specific // server (like MotorServer) from the ResourceManager and Server. Start the From eaad4b0345302a1e0e468c13f5998a71fa2c48ca Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Tue, 30 Jan 2024 10:26:46 -0500 Subject: [PATCH 05/20] everything compiling, all tests passing --- .../examples/modules/complex/gizmo/api.cpp | 4 ++++ .../examples/modules/complex/gizmo/api.hpp | 1 + .../examples/modules/complex/summation/api.cpp | 4 ++++ .../examples/modules/complex/summation/api.hpp | 1 + src/viam/sdk/components/base/server.cpp | 4 ++++ src/viam/sdk/components/base/server.hpp | 1 + src/viam/sdk/components/board/server.cpp | 4 ++++ src/viam/sdk/components/board/server.hpp | 1 + src/viam/sdk/components/camera/server.cpp | 4 ++++ src/viam/sdk/components/camera/server.hpp | 1 + src/viam/sdk/components/encoder/server.cpp | 4 ++++ src/viam/sdk/components/encoder/server.hpp | 1 + src/viam/sdk/components/generic/server.cpp | 4 ++++ src/viam/sdk/components/generic/server.hpp | 1 + src/viam/sdk/components/motor/server.cpp | 4 ++++ src/viam/sdk/components/motor/server.hpp | 1 + .../sdk/components/movement_sensor/server.cpp | 4 ++++ .../sdk/components/movement_sensor/server.hpp | 1 + .../sdk/components/power_sensor/server.cpp | 4 ++++ .../sdk/components/power_sensor/server.hpp | 1 + src/viam/sdk/components/sensor/server.cpp | 4 ++++ src/viam/sdk/components/sensor/server.hpp | 1 + src/viam/sdk/components/servo/server.cpp | 4 ++++ src/viam/sdk/components/servo/server.hpp | 1 + src/viam/sdk/registry/registry.hpp | 5 ++++- src/viam/sdk/resource/resource_server_base.cpp | 4 ++++ src/viam/sdk/resource/resource_server_base.hpp | 2 ++ src/viam/sdk/robot/service.cpp | 10 +++++++++- src/viam/sdk/robot/service.hpp | 2 ++ src/viam/sdk/rpc/server.cpp | 18 +++++++++++++++++- src/viam/sdk/rpc/server.hpp | 16 +++++++++++++--- src/viam/sdk/services/generic/server.cpp | 4 ++++ src/viam/sdk/services/generic/server.hpp | 1 + src/viam/sdk/services/mlmodel/server.cpp | 4 ++++ src/viam/sdk/services/mlmodel/server.hpp | 1 + src/viam/sdk/services/motion/server.cpp | 4 ++++ src/viam/sdk/services/motion/server.hpp | 1 + src/viam/sdk/tests/test_robot.cpp | 8 ++++---- src/viam/sdk/tests/test_utils.hpp | 12 +++++++++--- 39 files changed, 139 insertions(+), 13 deletions(-) diff --git a/src/viam/examples/modules/complex/gizmo/api.cpp b/src/viam/examples/modules/complex/gizmo/api.cpp index 1a061b6d1..e8d1618fb 100644 --- a/src/viam/examples/modules/complex/gizmo/api.cpp +++ b/src/viam/examples/modules/complex/gizmo/api.cpp @@ -45,6 +45,10 @@ std::shared_ptr Gizmo::resource_registration() { return std::make_shared(sd); } +API GizmoServer::api() const { + return API::get(); +} + API Gizmo::api() const { return API::get(); } diff --git a/src/viam/examples/modules/complex/gizmo/api.hpp b/src/viam/examples/modules/complex/gizmo/api.hpp index da4f9649b..b719093b1 100644 --- a/src/viam/examples/modules/complex/gizmo/api.hpp +++ b/src/viam/examples/modules/complex/gizmo/api.hpp @@ -71,6 +71,7 @@ class GizmoClient : public Gizmo { class GizmoServer : public ResourceServer, public GizmoService::Service { public: explicit GizmoServer(std::shared_ptr manager); + API api() const override; grpc::Status DoOne(grpc::ServerContext* context, const DoOneRequest* request, diff --git a/src/viam/examples/modules/complex/summation/api.cpp b/src/viam/examples/modules/complex/summation/api.cpp index bebf810a6..86dee4e20 100644 --- a/src/viam/examples/modules/complex/summation/api.cpp +++ b/src/viam/examples/modules/complex/summation/api.cpp @@ -50,6 +50,10 @@ API Summation::api() const { return API::get(); } +API SummationServer::api() const { + return API::get(); +} + API API::traits::api() { return {"viam", "service", "summation"}; } diff --git a/src/viam/examples/modules/complex/summation/api.hpp b/src/viam/examples/modules/complex/summation/api.hpp index dc166da31..8064f1fde 100644 --- a/src/viam/examples/modules/complex/summation/api.hpp +++ b/src/viam/examples/modules/complex/summation/api.hpp @@ -67,6 +67,7 @@ class SummationClient : public Summation { class SummationServer : public ResourceServer, public SummationService::Service { public: explicit SummationServer(std::shared_ptr manager); + API api() const override; grpc::Status Sum(grpc::ServerContext* context, const SumRequest* request, diff --git a/src/viam/sdk/components/base/server.cpp b/src/viam/sdk/components/base/server.cpp index b04080402..3a3b7260d 100644 --- a/src/viam/sdk/components/base/server.cpp +++ b/src/viam/sdk/components/base/server.cpp @@ -112,5 +112,9 @@ ::grpc::Status BaseServer::DoCommand(grpc::ServerContext* context, }); } +API BaseServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/base/server.hpp b/src/viam/sdk/components/base/server.hpp index 6755979ec..483d35744 100644 --- a/src/viam/sdk/components/base/server.hpp +++ b/src/viam/sdk/components/base/server.hpp @@ -19,6 +19,7 @@ namespace sdk { class BaseServer : public ResourceServer, public viam::component::base::v1::BaseService::Service { public: explicit BaseServer(std::shared_ptr manager); + API api() const override; ::grpc::Status MoveStraight( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/board/server.cpp b/src/viam/sdk/components/board/server.cpp index 5a218e244..d5504f6c8 100644 --- a/src/viam/sdk/components/board/server.cpp +++ b/src/viam/sdk/components/board/server.cpp @@ -204,5 +204,9 @@ ::grpc::Status BoardServer::GetGeometries( }); } +API BoardServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/board/server.hpp b/src/viam/sdk/components/board/server.hpp index 8bf5a6c39..361ac2e63 100644 --- a/src/viam/sdk/components/board/server.hpp +++ b/src/viam/sdk/components/board/server.hpp @@ -19,6 +19,7 @@ class BoardServer : public ResourceServer, public viam::component::board::v1::BoardService::Service { public: explicit BoardServer(std::shared_ptr manager); + API api() const override; ::grpc::Status Status(::grpc::ServerContext* context, const ::viam::component::board::v1::StatusRequest* request, diff --git a/src/viam/sdk/components/camera/server.cpp b/src/viam/sdk/components/camera/server.cpp index f4a594a0f..d792bea51 100644 --- a/src/viam/sdk/components/camera/server.cpp +++ b/src/viam/sdk/components/camera/server.cpp @@ -116,5 +116,9 @@ ::grpc::Status CameraServer::GetProperties( }); } +API CameraServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/camera/server.hpp b/src/viam/sdk/components/camera/server.hpp index a7ffdd3b2..703a9e411 100644 --- a/src/viam/sdk/components/camera/server.hpp +++ b/src/viam/sdk/components/camera/server.hpp @@ -19,6 +19,7 @@ class CameraServer : public ResourceServer, public viam::component::camera::v1::CameraService::Service { public: explicit CameraServer(std::shared_ptr manager); + API api() const override; ::grpc::Status DoCommand(::grpc::ServerContext* context, const ::viam::common::v1::DoCommandRequest* request, diff --git a/src/viam/sdk/components/encoder/server.cpp b/src/viam/sdk/components/encoder/server.cpp index 981127012..de47ec0c5 100644 --- a/src/viam/sdk/components/encoder/server.cpp +++ b/src/viam/sdk/components/encoder/server.cpp @@ -68,5 +68,9 @@ ::grpc::Status EncoderServer::DoCommand(grpc::ServerContext* context, }); } +API EncoderServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/encoder/server.hpp b/src/viam/sdk/components/encoder/server.hpp index c67c59e43..0c9b9f378 100644 --- a/src/viam/sdk/components/encoder/server.hpp +++ b/src/viam/sdk/components/encoder/server.hpp @@ -19,6 +19,7 @@ class EncoderServer : public ResourceServer, public viam::component::encoder::v1::EncoderService::Service { public: explicit EncoderServer(std::shared_ptr manager); + API api() const override; ::grpc::Status GetPosition( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/generic/server.cpp b/src/viam/sdk/components/generic/server.cpp index 95cfedeb7..1c580712d 100644 --- a/src/viam/sdk/components/generic/server.cpp +++ b/src/viam/sdk/components/generic/server.cpp @@ -33,5 +33,9 @@ ::grpc::Status GenericComponentServer::GetGeometries( }); } +API GenericComponentServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/generic/server.hpp b/src/viam/sdk/components/generic/server.hpp index d1061b9c0..7ca0d1c6e 100644 --- a/src/viam/sdk/components/generic/server.hpp +++ b/src/viam/sdk/components/generic/server.hpp @@ -19,6 +19,7 @@ class GenericComponentServer : public ResourceServer, public viam::component::generic::v1::GenericService::Service { public: explicit GenericComponentServer(std::shared_ptr manager); + API api() const override; ::grpc::Status DoCommand(::grpc::ServerContext* context, const ::viam::common::v1::DoCommandRequest* request, diff --git a/src/viam/sdk/components/motor/server.cpp b/src/viam/sdk/components/motor/server.cpp index 5ba2dbb89..5fbefe926 100644 --- a/src/viam/sdk/components/motor/server.cpp +++ b/src/viam/sdk/components/motor/server.cpp @@ -126,5 +126,9 @@ ::grpc::Status MotorServer::DoCommand(grpc::ServerContext* context, }); } +API MotorServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/motor/server.hpp b/src/viam/sdk/components/motor/server.hpp index 7e2a22c94..47ef2088b 100644 --- a/src/viam/sdk/components/motor/server.hpp +++ b/src/viam/sdk/components/motor/server.hpp @@ -19,6 +19,7 @@ class MotorServer : public ResourceServer, public viam::component::motor::v1::MotorService::Service { public: explicit MotorServer(std::shared_ptr manager); + API api() const override; ::grpc::Status SetPower( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/movement_sensor/server.cpp b/src/viam/sdk/components/movement_sensor/server.cpp index a38de44e9..05da89ad8 100644 --- a/src/viam/sdk/components/movement_sensor/server.cpp +++ b/src/viam/sdk/components/movement_sensor/server.cpp @@ -140,5 +140,9 @@ ::grpc::Status MovementSensorServer::GetGeometries( }); } +API MovementSensorServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/movement_sensor/server.hpp b/src/viam/sdk/components/movement_sensor/server.hpp index c0ad7e690..a4a46658f 100644 --- a/src/viam/sdk/components/movement_sensor/server.hpp +++ b/src/viam/sdk/components/movement_sensor/server.hpp @@ -20,6 +20,7 @@ class MovementSensorServer public viam::component::movementsensor::v1::MovementSensorService::Service { public: explicit MovementSensorServer(std::shared_ptr manager); + API api() const override; ::grpc::Status GetLinearVelocity( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/power_sensor/server.cpp b/src/viam/sdk/components/power_sensor/server.cpp index fb16abd8c..dc393702c 100644 --- a/src/viam/sdk/components/power_sensor/server.cpp +++ b/src/viam/sdk/components/power_sensor/server.cpp @@ -68,5 +68,9 @@ ::grpc::Status PowerSensorServer::DoCommand( }); } +API PowerSensorServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/power_sensor/server.hpp b/src/viam/sdk/components/power_sensor/server.hpp index ac6035612..d49bfcf2e 100644 --- a/src/viam/sdk/components/power_sensor/server.hpp +++ b/src/viam/sdk/components/power_sensor/server.hpp @@ -20,6 +20,7 @@ namespace sdk { class PowerSensorServer : public ResourceServer, public PowerSensorService::Service { public: explicit PowerSensorServer(std::shared_ptr manager); + API api() const override; ::grpc::Status GetVoltage(::grpc::ServerContext* context, const GetVoltageRequest* request, diff --git a/src/viam/sdk/components/sensor/server.cpp b/src/viam/sdk/components/sensor/server.cpp index eab3859aa..ba6231968 100644 --- a/src/viam/sdk/components/sensor/server.cpp +++ b/src/viam/sdk/components/sensor/server.cpp @@ -48,5 +48,9 @@ ::grpc::Status SensorServer::GetGeometries(::grpc::ServerContext* context, }); } +API SensorServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/sensor/server.hpp b/src/viam/sdk/components/sensor/server.hpp index 41baafbc9..0caaae7ad 100644 --- a/src/viam/sdk/components/sensor/server.hpp +++ b/src/viam/sdk/components/sensor/server.hpp @@ -21,6 +21,7 @@ class SensorServer : public ResourceServer, public viam::component::sensor::v1::SensorService::Service { public: explicit SensorServer(std::shared_ptr manager); + API api() const override; ::grpc::Status GetReadings(::grpc::ServerContext* context, const GetReadingsRequest* request, diff --git a/src/viam/sdk/components/servo/server.cpp b/src/viam/sdk/components/servo/server.cpp index 8ac254204..b4ad859a1 100644 --- a/src/viam/sdk/components/servo/server.cpp +++ b/src/viam/sdk/components/servo/server.cpp @@ -71,5 +71,9 @@ ::grpc::Status ServoServer::DoCommand(grpc::ServerContext* context, }); } +API ServoServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/servo/server.hpp b/src/viam/sdk/components/servo/server.hpp index e8ed6ff34..ad606d071 100644 --- a/src/viam/sdk/components/servo/server.hpp +++ b/src/viam/sdk/components/servo/server.hpp @@ -19,6 +19,7 @@ class ServoServer : public ResourceServer, public viam::component::servo::v1::ServoService::Service { public: explicit ServoServer(std::shared_ptr manager); + API api() const override; ::grpc::Status Move(::grpc::ServerContext* context, const ::viam::component::servo::v1::MoveRequest* request, diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index 96fd32aee..017c85d51 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -32,7 +32,10 @@ class ResourceRegistration { public: virtual ~ResourceRegistration(); - // CR erodkin: is this necessary at all? how do we use this? + // TODO(RSDK-6484): the semantics of `create_reconfigurable` are opaque (and to-date + // are in fact non-existent). We should get rid of this and just add a `Reconfigurable` + // base class (similar to `Stoppable`), and things that want to reconfigure can inherit + // from it. /// @brief Add `Reconfigure` functionality to a resource. std::function(std::shared_ptr, Name)> create_reconfigurable; diff --git a/src/viam/sdk/resource/resource_server_base.cpp b/src/viam/sdk/resource/resource_server_base.cpp index 9f966680a..d80b225ce 100644 --- a/src/viam/sdk/resource/resource_server_base.cpp +++ b/src/viam/sdk/resource/resource_server_base.cpp @@ -6,5 +6,9 @@ const std::shared_ptr& ResourceServer::resource_manager() const return manager_; }; +std::shared_ptr& ResourceServer::resource_manager() { + return manager_; +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/resource/resource_server_base.hpp b/src/viam/sdk/resource/resource_server_base.hpp index a7a954090..581b9a9b8 100644 --- a/src/viam/sdk/resource/resource_server_base.hpp +++ b/src/viam/sdk/resource/resource_server_base.hpp @@ -7,7 +7,9 @@ namespace sdk { class ResourceServer { public: + virtual API api() const = 0; const std::shared_ptr& resource_manager() const; + std::shared_ptr& resource_manager(); protected: ResourceServer(std::shared_ptr manager) : manager_(manager){}; diff --git a/src/viam/sdk/robot/service.cpp b/src/viam/sdk/robot/service.cpp index 83b25c81c..94d3328fe 100644 --- a/src/viam/sdk/robot/service.cpp +++ b/src/viam/sdk/robot/service.cpp @@ -31,8 +31,12 @@ using viam::common::v1::ResourceName; using viam::robot::v1::Status; RobotService_::RobotService_(std::shared_ptr manager, Server& server) - : ResourceServer(std::move(manager)) { + : ResourceServer(manager) { server.register_service(this); + // register all managed resources with the appropriate resource servers. + for (const auto& resource : manager->resources()) { + server.add_resource(resource.second); + } } std::vector RobotService_::generate_metadata() { @@ -209,5 +213,9 @@ std::shared_ptr RobotService_::resource_by_name(Name name) { return r; } +API RobotService_::api() const { + return {"RDK", "robot", "robot"}; +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/robot/service.hpp b/src/viam/sdk/robot/service.hpp index 2864b2caa..10fa6729c 100644 --- a/src/viam/sdk/robot/service.hpp +++ b/src/viam/sdk/robot/service.hpp @@ -49,6 +49,8 @@ class RobotService_ : public ResourceServer, public viam::robot::v1::RobotServic ::viam::robot::v1::StopAllResponse* response) override; private: + // CR erodkin: this is a bit of a hacky workaround, leave a comment about it + API api() const override; std::mutex lock_; std::vector generate_metadata(); std::vector generate_status(RepeatedPtrField resource_names); diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 30b28bf0f..401a5c0a8 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -15,7 +15,7 @@ Server::Server() : builder_(std::make_unique()) { // have a server and that server can track everything for them. But we'll need to test that! for (const auto& rr : Registry::registered_resources()) { auto server = rr.second->create_resource_server(new_manager, *this); - managed_servers_.push_back(server); + managed_servers_.emplace(server->api(), std::move(server)); } } @@ -31,6 +31,22 @@ void Server::register_service(grpc::Service* service) { builder_->RegisterService(service); } +void Server::add_resource(std::shared_ptr resource) { + if (!builder_) { + throw std::runtime_error("Cannot add a new resource after the server has started"); + } + + auto api = resource->api(); + if (managed_servers_.find(api) == managed_servers_.end()) { + std::ostringstream buffer; + buffer << "Attempted to add resource with API: " << api + << " but no matching resource server as found"; + throw std::runtime_error(buffer.str()); + } + auto resource_server = managed_servers_.at(std::move(api)); + resource_server->resource_manager()->add(resource->name(), std::move(resource)); +} + void Server::start() { if (server_) { throw std::runtime_error("Attempted to start server that was already running"); diff --git a/src/viam/sdk/rpc/server.hpp b/src/viam/sdk/rpc/server.hpp index 7d8b18fa1..540668c49 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -7,6 +7,8 @@ #include #include +#include +#include #include namespace viam { @@ -30,13 +32,21 @@ class Server { /// repeated calls. void start(); - // TODO: make `register_service` take one of our types as an arg rather than a - // grpc service type, and convert under the hood + // CR erodkin: deleted a todo here because it wouldn't play nice with user-defined modular + // resources. Leave a flyby comment to that effect in the PR. /// @brief Registers a gRPC service. /// @param service The gRPC service to be registered. /// @throws `std::runtime_error` if called after the server has been `start`ed. void register_service(grpc::Service* service); + // CR erodkin: I don't love having this on server. Consider if we can put this somewhere + // else? + /// @brief Adds a specific managed resource to the associated resource server + /// @param resource The resource to add + /// @throws `std::runtime_error` if a matching `ResourceServer` doesn't exist in the server, + /// or if called after the server has `start`ed. + void add_resource(std::shared_ptr resource); + /// @brief Adds a listening port to the server. /// @param address The address to listen at. /// @param creds The server credentials; defaults to a insecure server credentials. @@ -54,7 +64,7 @@ class Server { friend class ::viam::sdktests::TestServer; private: - std::vector> managed_servers_; + std::unordered_map> managed_servers_; std::unique_ptr builder_; std::unique_ptr server_; }; diff --git a/src/viam/sdk/services/generic/server.cpp b/src/viam/sdk/services/generic/server.cpp index 5403d41b3..ea983745b 100644 --- a/src/viam/sdk/services/generic/server.cpp +++ b/src/viam/sdk/services/generic/server.cpp @@ -21,5 +21,9 @@ ::grpc::Status GenericServiceServer::DoCommand( }); } +API GenericServiceServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/services/generic/server.hpp b/src/viam/sdk/services/generic/server.hpp index f72ab2b94..b98d48a0a 100644 --- a/src/viam/sdk/services/generic/server.hpp +++ b/src/viam/sdk/services/generic/server.hpp @@ -19,6 +19,7 @@ class GenericServiceServer : public ResourceServer, public viam::service::generic::v1::GenericService::Service { public: explicit GenericServiceServer(std::shared_ptr manager); + API api() const override; ::grpc::Status DoCommand(::grpc::ServerContext* context, const ::viam::common::v1::DoCommandRequest* request, diff --git a/src/viam/sdk/services/mlmodel/server.cpp b/src/viam/sdk/services/mlmodel/server.cpp index 50dbfb37b..3a269f0bf 100644 --- a/src/viam/sdk/services/mlmodel/server.cpp +++ b/src/viam/sdk/services/mlmodel/server.cpp @@ -158,5 +158,9 @@ ::grpc::Status MLModelServiceServer::Metadata( }); } +API MLModelServiceServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/services/mlmodel/server.hpp b/src/viam/sdk/services/mlmodel/server.hpp index e4ac1f8b8..7679ced29 100644 --- a/src/viam/sdk/services/mlmodel/server.hpp +++ b/src/viam/sdk/services/mlmodel/server.hpp @@ -30,6 +30,7 @@ class MLModelServiceServer : public ResourceServer, public ::viam::service::mlmodel::v1::MLModelService::Service { public: explicit MLModelServiceServer(std::shared_ptr manager); + API api() const override; ::grpc::Status Infer(::grpc::ServerContext* context, const ::viam::service::mlmodel::v1::InferRequest* request, diff --git a/src/viam/sdk/services/motion/server.cpp b/src/viam/sdk/services/motion/server.cpp index 94edd590b..20580fdc3 100644 --- a/src/viam/sdk/services/motion/server.cpp +++ b/src/viam/sdk/services/motion/server.cpp @@ -212,5 +212,9 @@ ::grpc::Status MotionServer::DoCommand(::grpc::ServerContext* context, }); }; +API MotionServer::api() const { + return API::get(); +} + } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/services/motion/server.hpp b/src/viam/sdk/services/motion/server.hpp index cc145f4fe..1db6d8b4d 100644 --- a/src/viam/sdk/services/motion/server.hpp +++ b/src/viam/sdk/services/motion/server.hpp @@ -21,6 +21,7 @@ class MotionServer : public ResourceServer, public viam::service::motion::v1::MotionService::Service { public: explicit MotionServer(std::shared_ptr manager); + API api() const override; ::grpc::Status Move(::grpc::ServerContext* context, const ::viam::service::motion::v1::MoveRequest* request, diff --git a/src/viam/sdk/tests/test_robot.cpp b/src/viam/sdk/tests/test_robot.cpp index 486c52141..9ee1f13b2 100644 --- a/src/viam/sdk/tests/test_robot.cpp +++ b/src/viam/sdk/tests/test_robot.cpp @@ -49,11 +49,8 @@ void robot_client_to_mocks_pipeline(F&& test_case) { rm->add(std::string("mock_motor"), motor::MockMotor::get_mock_motor()); rm->add(std::string("mock_camera"), camera::MockCamera::get_mock_camera()); auto server = std::make_shared(); - std::cout << "gonna create a robot service\n" << std::flush; MockRobotService service(rm, *server); - std::cout << "gonna start a robot service\n" << std::flush; server->start(); - std::cout << " started a robot service\n" << std::flush; // Create a RobotClient to the MockRobotService over an established // in-process gRPC channel. @@ -226,7 +223,10 @@ BOOST_AUTO_TEST_CASE(test_get_resource) { [](std::shared_ptr client, MockRobotService& service) -> void { auto mock_motor = client->resource_by_name("mock_motor"); BOOST_CHECK(mock_motor); - // BOOST_CHECK(!mock_motor->is_moving()); + + // test not just that we can get the motor, but that the motor service has been + // appropriately registered such that we can actually use it. + BOOST_CHECK(!mock_motor->is_moving()); }); } diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index 8fc205412..4724bbe48 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -56,14 +56,20 @@ void client_to_mock_pipeline(std::shared_ptr mock, F&& test_case) { // Create a Server. Use the mock's API to create a resource-specific // server (like MotorServer) from the ResourceManager and Server. Start the // Server. - auto rm = std::make_shared(); - rm->add(mock->name(), mock); + // + // CR erodkin: commented code should be deleted BUT we might want to go back to this kind + // of pattern if we do JIT server registration (as opposed to registering all resource + // servers when we start the RPC server, as we do now). So keeping it around just in case. + // + // auto rm = std::make_shared(); + // rm->add(mock->name(), mock); auto server = std::make_shared(); + server->add_resource(mock); auto rs = sdk::Registry::lookup_resource(mock->api()); // resource_server is unused; we call create_resource_server to call // register_service and associate the Server with the resource-specific // server (like MotorServer). - auto resource_server = rs->create_resource_server(rm, *server); + // auto resource_server = rs->create_resource_server(rm, *server); server->start(); // Create a resource-specific client to the mock over an established From 58b2515ea95cb101f98381dff40de821d2760b09 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 31 Jan 2024 09:19:33 -0500 Subject: [PATCH 06/20] figured out duplicate registration but gizmo not working? --- .../dial_api_key/example_dial_api_key.cpp | 6 +-- src/viam/examples/modules/complex/client.cpp | 19 +++++-- .../examples/modules/complex/gizmo/api.cpp | 2 +- src/viam/examples/modules/complex/main.cpp | 9 +++- .../modules/complex/summation/api.cpp | 54 +++++++++---------- .../modules/complex/summation/api.hpp | 18 +++---- .../modules/complex/test_complex_module.cpp | 4 +- src/viam/sdk/module/service.cpp | 17 ++++-- src/viam/sdk/registry/registry.cpp | 2 + src/viam/sdk/rpc/server.cpp | 19 ++++++- 10 files changed, 97 insertions(+), 53 deletions(-) diff --git a/src/viam/examples/dial_api_key/example_dial_api_key.cpp b/src/viam/examples/dial_api_key/example_dial_api_key.cpp index 33391e18b..2ba11ad80 100644 --- a/src/viam/examples/dial_api_key/example_dial_api_key.cpp +++ b/src/viam/examples/dial_api_key/example_dial_api_key.cpp @@ -26,11 +26,11 @@ using viam::robot::v1::Status; using namespace viam::sdk; int main() { - const char* uri = ""; + const char* uri = "webrtc-test-main.jkek76kqnh.viam.cloud"; DialOptions dial_options; std::string type = "api-key"; - std::string entity = ""; - std::string payload = ""; + std::string entity = "a9dcf212-3397-4318-bb1e-f5c36b3cafc1"; + std::string payload = "2ml60ys1j4i9v0pecpxahe654yxpc1dz"; dial_options.set_entity(entity); Credentials credentials(type, payload); dial_options.set_credentials(credentials); diff --git a/src/viam/examples/modules/complex/client.cpp b/src/viam/examples/modules/complex/client.cpp index d658ceef4..7e38cc318 100644 --- a/src/viam/examples/modules/complex/client.cpp +++ b/src/viam/examples/modules/complex/client.cpp @@ -27,25 +27,32 @@ using namespace viam::sdk; int main() { - const char* uri = ""; + const char* uri = "webrtc-test-main.jkek76kqnh.viam.cloud"; DialOptions dial_options; - std::string type = ""; - std::string payload = ""; + std::string type = "api-key"; + std::string entity = "a9dcf212-3397-4318-bb1e-f5c36b3cafc1"; + std::string payload = "2ml60ys1j4i9v0pecpxahe654yxpc1dz"; + dial_options.set_entity(entity); Credentials credentials(type, payload); dial_options.set_credentials(credentials); boost::optional opts(dial_options); std::string address(uri); Options options(1, opts); + std::cout << "doing a thing" << std::flush; // initialize the Registry to ensure all built-in components and gRPC server reflection // are supported. - Registry::initialize(); + // Registry::initialize(); + std::cout << "inited registry\n" << std::flush; // Register custom gizmo and summation API so robot client can access resources // of that type from the server. Registry::register_resource(API::get(), Gizmo::resource_registration()); - Registry::register_resource(API::get(), Summation::resource_registration()); + Registry::register_resource( + API::get()); + // Registry::register_resource(API::get(), Summation::resource_registration()); + std::cout << "registered resources\n" << std::flush; // Connect to robot. std::shared_ptr robot = RobotClient::at_address(address, options); // Print resources. @@ -62,6 +69,8 @@ int main() { std::cerr << "could not get 'gizmo1' resource from robot" << std::endl; return EXIT_FAILURE; } + + std::cout << "trying to do a thing " << std::endl; bool do_one_ret = gc->do_one("arg1"); std::cout << "gizmo1 do_one returned: " << do_one_ret << std::endl; bool do_one_client_stream_ret = gc->do_one_client_stream({"arg1", "arg1", "arg1"}); diff --git a/src/viam/examples/modules/complex/gizmo/api.cpp b/src/viam/examples/modules/complex/gizmo/api.cpp index e8d1618fb..68657b381 100644 --- a/src/viam/examples/modules/complex/gizmo/api.cpp +++ b/src/viam/examples/modules/complex/gizmo/api.cpp @@ -24,7 +24,7 @@ GizmoRegistration::GizmoRegistration(const google::protobuf::ServiceDescriptor* std::shared_ptr GizmoRegistration::create_resource_server( std::shared_ptr manager, Server& server) { auto gs = std::make_shared(std::move(manager)); - server.register_service(gs.get()); + // server.register_service(gs.get()); return gs; }; diff --git a/src/viam/examples/modules/complex/main.cpp b/src/viam/examples/modules/complex/main.cpp index 4ab7cb662..f529371ea 100644 --- a/src/viam/examples/modules/complex/main.cpp +++ b/src/viam/examples/modules/complex/main.cpp @@ -28,6 +28,8 @@ using namespace viam::sdk; int main(int argc, char** argv) { + std::cout << "at beginning!" << std::endl; + Registry::initialize(); API base_api = API::get(); Model mybase_model("viam", "base", "mybase"); @@ -42,6 +44,7 @@ int main(int argc, char** argv) { // Make sure to explicity register resources with custom APIs. Note that // this must be done in `main` and not in resource implementation files due // to order of static initialization. + std::cout << "initialized registry, about to register gizmo" << std::endl; Registry::register_resource(gizmo_api, Gizmo::resource_registration()); std::shared_ptr mygizmo_mr = std::make_shared( gizmo_api, @@ -54,7 +57,9 @@ int main(int argc, char** argv) { // Make sure to explicity register resources with custom APIs. Note that // this must be done in `main` and not in resource implementation files due // to order of static initialization. - Registry::register_resource(summation_api, Summation::resource_registration()); + std::cout << "registered gizmo, about to register summation" << std::endl; + Registry::register_resource(summation_api); + // Registry::register_resource(summation_api, Summation::resource_registration()); std::shared_ptr mysummation_mr = std::make_shared( summation_api, mysummation_model, [](Dependencies deps, ResourceConfig cfg) { @@ -62,7 +67,9 @@ int main(int argc, char** argv) { }); std::vector> mrs = {mybase_mr, mygizmo_mr, mysummation_mr}; + std::cout << "creating modules service " << std::endl; auto my_mod = std::make_shared(argc, argv, mrs); + std::cout << "serving module " << std::endl; my_mod->serve(); return EXIT_SUCCESS; diff --git a/src/viam/examples/modules/complex/summation/api.cpp b/src/viam/examples/modules/complex/summation/api.cpp index 86dee4e20..74f966073 100644 --- a/src/viam/examples/modules/complex/summation/api.cpp +++ b/src/viam/examples/modules/complex/summation/api.cpp @@ -18,33 +18,33 @@ using namespace viam::service::summation::v1; /* SummationRegistration methods */ -SummationRegistration::SummationRegistration( - const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor){}; - -std::shared_ptr SummationRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto ss = std::make_shared(std::move(manager)); - server.register_service(ss.get()); - return ss; -}; - -std::shared_ptr SummationRegistration::create_rpc_client( - std::string name, std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -}; - -/* Summation methods */ - -std::shared_ptr Summation::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(SummationService::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor for the summation service"); - } - return std::make_shared(sd); -} +// SummationRegistration::SummationRegistration( +// const google::protobuf::ServiceDescriptor* service_descriptor) +//: ResourceRegistration(service_descriptor){}; + +// std::shared_ptr SummationRegistration::create_resource_server( +// std::shared_ptr manager, Server& server) { +// auto ss = std::make_shared(std::move(manager)); +// server.register_service(ss.get()); +// return ss; +//}; + +// std::shared_ptr SummationRegistration::create_rpc_client( +// std::string name, std::shared_ptr chan) { +// return std::make_shared(std::move(name), std::move(chan)); +//}; + +//[> Summation methods <] + +// std::shared_ptr Summation::resource_registration() { +// const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); +// const google::protobuf::ServiceDescriptor* sd = +// p->FindServiceByName(SummationService::service_full_name()); +// if (!sd) { +// throw std::runtime_error("Unable to get service descriptor for the summation service"); +//} +// return std::make_shared(sd); +//} API Summation::api() const { return API::get(); diff --git a/src/viam/examples/modules/complex/summation/api.hpp b/src/viam/examples/modules/complex/summation/api.hpp index 8064f1fde..c8a6a6c47 100644 --- a/src/viam/examples/modules/complex/summation/api.hpp +++ b/src/viam/examples/modules/complex/summation/api.hpp @@ -19,20 +19,20 @@ using namespace viam::service::summation::v1; // `SummationRegistration` defines a `ResourceRegistration` for the `Summation` // service. -class SummationRegistration : public ResourceRegistration { - public: - explicit SummationRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; +// class SummationRegistration : public ResourceRegistration { +// public: +// explicit SummationRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); +// std::shared_ptr create_resource_server(std::shared_ptr manager, +// Server& server) override; +// std::shared_ptr create_rpc_client(std::string name, +// std::shared_ptr chan) override; +//}; // A `Summation` is a custom modular service. class Summation : public Service { public: // methods shared across all services - static std::shared_ptr resource_registration(); + // static std::shared_ptr resource_registration(); API api() const override; virtual double sum(std::vector numbers) = 0; diff --git a/src/viam/examples/modules/complex/test_complex_module.cpp b/src/viam/examples/modules/complex/test_complex_module.cpp index 7bd3b037f..86fbde666 100644 --- a/src/viam/examples/modules/complex/test_complex_module.cpp +++ b/src/viam/examples/modules/complex/test_complex_module.cpp @@ -27,7 +27,9 @@ using namespace viam::sdktests; struct RegisterGizmoAndSummationFixture { RegisterGizmoAndSummationFixture() { Registry::register_resource(API::get(), Gizmo::resource_registration()); - Registry::register_resource(API::get(), Summation::resource_registration()); + Registry::register_resource( + API::get()); + // Registry::register_resource(API::get(), Summation::resource_registration()); } // Test teardown is a noop; diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index ccae04204..78c0f7437 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -210,7 +210,9 @@ ::grpc::Status ModuleService::Ready(::grpc::ServerContext* context, }; ModuleService::ModuleService(std::string addr) - : module_(std::make_unique(std::move(addr))), server_(std::make_unique()) {} + : module_(std::make_unique(std::move(addr))), server_(std::make_unique()) { + std::cout << "starting module service\n" << std::flush; +} ModuleService::ModuleService(int argc, char** argv, @@ -235,11 +237,14 @@ void ModuleService::serve() { listen(sockfd, 10); umask(old_mask); + std::cout << "registering service " << std::endl; server_->register_service(this); const std::string address = "unix://" + module_->addr(); server_->add_listening_port(address); + std::cout << "starting server " << std::endl; module_->set_ready(); + std::cout << "starting server2" << std::endl; server_->start(); BOOST_LOG_TRIVIAL(info) << "Module listening on " << module_->addr(); @@ -264,17 +269,21 @@ ModuleService::~ModuleService() { } void ModuleService::add_api_from_registry_inlock_(API api, const std::lock_guard&) { + std::cout << "calling this thing with api " << api << std::endl; const std::unordered_map>& services = module_->services(); if (services.find(api) != services.end()) { return; } auto new_manager = std::make_shared(); + std::cout << "the API we're dealing with is " << api << std::endl; const std::shared_ptr rs = Registry::lookup_resource(api); - const std::shared_ptr resource_server = - rs->create_resource_server(new_manager, *server_); + // // CR erodkin: I think this is where the issue was? we created resource server here and + // also when creating the server. + // const std::shared_ptr resource_server = + // rs->create_resource_server(new_manager, *server_); module_->mutable_services().emplace(api, new_manager); - module_->mutable_servers().push_back(resource_server); + // module_->mutable_servers().push_back(resource_server); } void ModuleService::add_model_from_registry_inlock_(API api, diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 4d35cd719..6d4b16623 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -157,8 +157,10 @@ const google::protobuf::ServiceDescriptor* ResourceRegistration::service_descrip void register_resources() { // Register all components + std::cout << "registering base!!" << std::endl; Registry::register_resource( API::get()); + std::cout << "registered base!!" << std::endl; Registry::register_resource( API::get()); Registry::register_resource( diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 401a5c0a8..70ef374fa 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -1,5 +1,6 @@ #include +#include #include #include @@ -8,11 +9,16 @@ namespace viam { namespace sdk { Server::Server() : builder_(std::make_unique()) { + std::cout << "creating a new server! this should only happen once!" << std::endl; auto new_manager = std::make_shared(); // CR erodkin: is this the best place to do this? Probably it makes more sense when we actually // register the service. see if we can figure out how to do that. Related: if we can do it right // then we don't need to manage resources separately in the module manager probably? it'll also // have a server and that server can track everything for them. But we'll need to test that! + // + // CR erodkin: is it possible we have a duplicate in the registered resources somehow and that's + // what is causing the issue? Look into that possibility and/or the above CR as a separate + // implementation for (const auto& rr : Registry::registered_resources()) { auto server = rr.second->create_resource_server(new_manager, *this); managed_servers_.emplace(server->api(), std::move(server)); @@ -27,8 +33,17 @@ void Server::register_service(grpc::Service* service) { if (!builder_) { throw std::runtime_error("Cannot register a new service after the server has started"); } - - builder_->RegisterService(service); + std::cout << "registering a service! " << std::endl; + + try { + builder_->RegisterService(service); + } catch (const std::runtime_error& e) { + std::cout << "caught an error!\n" << std::flush; + BOOST_LOG_TRIVIAL(warning) << "Failed to register service: " << e.what(); + } catch (...) { + std::cout << "caught an error!\n" << std::flush; + BOOST_LOG_TRIVIAL(warning) << "Failed to register service with unknown error"; + } } void Server::add_resource(std::shared_ptr resource) { From a0c47e4930e78b6ee0d09ad6dce1b8ebd81ddee1 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 31 Jan 2024 10:23:50 -0500 Subject: [PATCH 07/20] cleanup, no regression --- src/viam/examples/modules/complex/client.cpp | 9 ++---- .../examples/modules/complex/gizmo/api.cpp | 31 +------------------ .../examples/modules/complex/gizmo/api.hpp | 13 -------- src/viam/examples/modules/complex/main.cpp | 24 +++++--------- .../modules/complex/summation/api.cpp | 30 ------------------ .../modules/complex/summation/api.hpp | 12 ------- .../modules/complex/test_complex_module.cpp | 3 +- src/viam/sdk/module/service.cpp | 23 +++++--------- src/viam/sdk/rpc/server.cpp | 30 ++++++------------ src/viam/sdk/rpc/server.hpp | 9 +++++- 10 files changed, 39 insertions(+), 145 deletions(-) diff --git a/src/viam/examples/modules/complex/client.cpp b/src/viam/examples/modules/complex/client.cpp index 7e38cc318..4f6adffb5 100644 --- a/src/viam/examples/modules/complex/client.cpp +++ b/src/viam/examples/modules/complex/client.cpp @@ -38,16 +38,11 @@ int main() { boost::optional opts(dial_options); std::string address(uri); Options options(1, opts); - std::cout << "doing a thing" << std::flush; - // initialize the Registry to ensure all built-in components and gRPC server reflection - // are supported. - // Registry::initialize(); - - std::cout << "inited registry\n" << std::flush; // Register custom gizmo and summation API so robot client can access resources // of that type from the server. - Registry::register_resource(API::get(), Gizmo::resource_registration()); + Registry::register_resource(API::get()); + // Registry::register_resource(API::get(), Gizmo::resource_registration()); Registry::register_resource( API::get()); // Registry::register_resource(API::get(), Summation::resource_registration()); diff --git a/src/viam/examples/modules/complex/gizmo/api.cpp b/src/viam/examples/modules/complex/gizmo/api.cpp index 68657b381..733a7e8d8 100644 --- a/src/viam/examples/modules/complex/gizmo/api.cpp +++ b/src/viam/examples/modules/complex/gizmo/api.cpp @@ -16,35 +16,6 @@ using namespace viam::sdk; using namespace viam::component::gizmo::v1; -/* GizmoRegistration methods */ - -GizmoRegistration::GizmoRegistration(const google::protobuf::ServiceDescriptor* service_descriptor) - : ResourceRegistration(service_descriptor){}; - -std::shared_ptr GizmoRegistration::create_resource_server( - std::shared_ptr manager, Server& server) { - auto gs = std::make_shared(std::move(manager)); - // server.register_service(gs.get()); - return gs; -}; - -std::shared_ptr GizmoRegistration::create_rpc_client( - std::string name, std::shared_ptr chan) { - return std::make_shared(std::move(name), std::move(chan)); -}; - -/* Gizmo methods */ - -std::shared_ptr Gizmo::resource_registration() { - const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); - const google::protobuf::ServiceDescriptor* sd = - p->FindServiceByName(GizmoService::service_full_name()); - if (!sd) { - throw std::runtime_error("Unable to get service descriptor for the gizmo service"); - } - return std::make_shared(sd); -} - API GizmoServer::api() const { return API::get(); } @@ -72,7 +43,7 @@ grpc::Status GizmoServer::DoOne(grpc::ServerContext* context, "Called [Gizmo::DoOne] without a request"); }; - auto rg = ResourceServer::resource_manager()->resource(request->name()); + auto rg = resource_manager()->resource(request->name()); if (!rg) { return grpc::Status(grpc::UNKNOWN, "resource not found: " + request->name()); } diff --git a/src/viam/examples/modules/complex/gizmo/api.hpp b/src/viam/examples/modules/complex/gizmo/api.hpp index b719093b1..834c5b3ec 100644 --- a/src/viam/examples/modules/complex/gizmo/api.hpp +++ b/src/viam/examples/modules/complex/gizmo/api.hpp @@ -15,22 +15,9 @@ using namespace viam::sdk; using namespace viam::component::gizmo::v1; -// `GizmoRegistration` Defines a `ResourceRegistration` for the `Gizmo` -// component. -class GizmoRegistration : public ResourceRegistration { - public: - explicit GizmoRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); - std::shared_ptr create_resource_server(std::shared_ptr manager, - Server& server) override; - std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr chan) override; -}; - // `Gizmo` is a custom modular component. class Gizmo : public Component { public: - // methods shared across all components - static std::shared_ptr resource_registration(); API api() const override; virtual bool do_one(std::string arg1) = 0; diff --git a/src/viam/examples/modules/complex/main.cpp b/src/viam/examples/modules/complex/main.cpp index f529371ea..ce9d1219f 100644 --- a/src/viam/examples/modules/complex/main.cpp +++ b/src/viam/examples/modules/complex/main.cpp @@ -28,38 +28,32 @@ using namespace viam::sdk; int main(int argc, char** argv) { - std::cout << "at beginning!" << std::endl; Registry::initialize(); API base_api = API::get(); + API gizmo_api = API::get(); + API summation_api = API::get(); Model mybase_model("viam", "base", "mybase"); + // // CR erodkin: add note about how we've reduced the comment here. + // Make sure to explicity register resources with custom APIs. + Registry::register_resource(gizmo_api); + Registry::register_resource(summation_api); + std::shared_ptr mybase_mr = std::make_shared( base_api, mybase_model, [](Dependencies deps, ResourceConfig cfg) { return std::make_unique(deps, cfg); }, MyBase::validate); - API gizmo_api = API::get(); Model mygizmo_model("viam", "gizmo", "mygizmo"); - // Make sure to explicity register resources with custom APIs. Note that - // this must be done in `main` and not in resource implementation files due - // to order of static initialization. - std::cout << "initialized registry, about to register gizmo" << std::endl; - Registry::register_resource(gizmo_api, Gizmo::resource_registration()); + // Make sure to explicity register resources with custom APIs. std::shared_ptr mygizmo_mr = std::make_shared( gizmo_api, mygizmo_model, [](Dependencies deps, ResourceConfig cfg) { return std::make_unique(deps, cfg); }, MyGizmo::validate); - API summation_api = API::get(); Model mysummation_model("viam", "summation", "mysummation"); - // Make sure to explicity register resources with custom APIs. Note that - // this must be done in `main` and not in resource implementation files due - // to order of static initialization. - std::cout << "registered gizmo, about to register summation" << std::endl; - Registry::register_resource(summation_api); - // Registry::register_resource(summation_api, Summation::resource_registration()); std::shared_ptr mysummation_mr = std::make_shared( summation_api, mysummation_model, [](Dependencies deps, ResourceConfig cfg) { @@ -67,9 +61,7 @@ int main(int argc, char** argv) { }); std::vector> mrs = {mybase_mr, mygizmo_mr, mysummation_mr}; - std::cout << "creating modules service " << std::endl; auto my_mod = std::make_shared(argc, argv, mrs); - std::cout << "serving module " << std::endl; my_mod->serve(); return EXIT_SUCCESS; diff --git a/src/viam/examples/modules/complex/summation/api.cpp b/src/viam/examples/modules/complex/summation/api.cpp index 74f966073..b9f3dc6d1 100644 --- a/src/viam/examples/modules/complex/summation/api.cpp +++ b/src/viam/examples/modules/complex/summation/api.cpp @@ -16,36 +16,6 @@ using namespace viam::sdk; using namespace viam::service::summation::v1; -/* SummationRegistration methods */ - -// SummationRegistration::SummationRegistration( -// const google::protobuf::ServiceDescriptor* service_descriptor) -//: ResourceRegistration(service_descriptor){}; - -// std::shared_ptr SummationRegistration::create_resource_server( -// std::shared_ptr manager, Server& server) { -// auto ss = std::make_shared(std::move(manager)); -// server.register_service(ss.get()); -// return ss; -//}; - -// std::shared_ptr SummationRegistration::create_rpc_client( -// std::string name, std::shared_ptr chan) { -// return std::make_shared(std::move(name), std::move(chan)); -//}; - -//[> Summation methods <] - -// std::shared_ptr Summation::resource_registration() { -// const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); -// const google::protobuf::ServiceDescriptor* sd = -// p->FindServiceByName(SummationService::service_full_name()); -// if (!sd) { -// throw std::runtime_error("Unable to get service descriptor for the summation service"); -//} -// return std::make_shared(sd); -//} - API Summation::api() const { return API::get(); } diff --git a/src/viam/examples/modules/complex/summation/api.hpp b/src/viam/examples/modules/complex/summation/api.hpp index c8a6a6c47..1118732fb 100644 --- a/src/viam/examples/modules/complex/summation/api.hpp +++ b/src/viam/examples/modules/complex/summation/api.hpp @@ -17,22 +17,10 @@ using namespace viam::sdk; using namespace viam::service::summation::v1; -// `SummationRegistration` defines a `ResourceRegistration` for the `Summation` -// service. -// class SummationRegistration : public ResourceRegistration { -// public: -// explicit SummationRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); -// std::shared_ptr create_resource_server(std::shared_ptr manager, -// Server& server) override; -// std::shared_ptr create_rpc_client(std::string name, -// std::shared_ptr chan) override; -//}; - // A `Summation` is a custom modular service. class Summation : public Service { public: // methods shared across all services - // static std::shared_ptr resource_registration(); API api() const override; virtual double sum(std::vector numbers) = 0; diff --git a/src/viam/examples/modules/complex/test_complex_module.cpp b/src/viam/examples/modules/complex/test_complex_module.cpp index 86fbde666..4e9ca0b5c 100644 --- a/src/viam/examples/modules/complex/test_complex_module.cpp +++ b/src/viam/examples/modules/complex/test_complex_module.cpp @@ -26,7 +26,8 @@ using namespace viam::sdktests; struct RegisterGizmoAndSummationFixture { RegisterGizmoAndSummationFixture() { - Registry::register_resource(API::get(), Gizmo::resource_registration()); + Registry::register_resource(API::get()); + // Registry::register_resource(API::get(), Gizmo::resource_registration()); Registry::register_resource( API::get()); // Registry::register_resource(API::get(), Summation::resource_registration()); diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 78c0f7437..4fb625ea6 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -210,9 +210,7 @@ ::grpc::Status ModuleService::Ready(::grpc::ServerContext* context, }; ModuleService::ModuleService(std::string addr) - : module_(std::make_unique(std::move(addr))), server_(std::make_unique()) { - std::cout << "starting module service\n" << std::flush; -} + : module_(std::make_unique(std::move(addr))), server_(std::make_unique()) {} ModuleService::ModuleService(int argc, char** argv, @@ -237,14 +235,11 @@ void ModuleService::serve() { listen(sockfd, 10); umask(old_mask); - std::cout << "registering service " << std::endl; server_->register_service(this); const std::string address = "unix://" + module_->addr(); server_->add_listening_port(address); - std::cout << "starting server " << std::endl; module_->set_ready(); - std::cout << "starting server2" << std::endl; server_->start(); BOOST_LOG_TRIVIAL(info) << "Module listening on " << module_->addr(); @@ -269,21 +264,19 @@ ModuleService::~ModuleService() { } void ModuleService::add_api_from_registry_inlock_(API api, const std::lock_guard&) { - std::cout << "calling this thing with api " << api << std::endl; const std::unordered_map>& services = module_->services(); if (services.find(api) != services.end()) { return; } - auto new_manager = std::make_shared(); - std::cout << "the API we're dealing with is " << api << std::endl; const std::shared_ptr rs = Registry::lookup_resource(api); - // // CR erodkin: I think this is where the issue was? we created resource server here and - // also when creating the server. - // const std::shared_ptr resource_server = - // rs->create_resource_server(new_manager, *server_); - module_->mutable_services().emplace(api, new_manager); - // module_->mutable_servers().push_back(resource_server); + auto server = server_->lookup_resource_server(api); + // CR erodkin: instead of having module hold on to copies of the services/servers, just + // point to what's in the server when doing add operations + if (server) { + module_->mutable_services().emplace(api, server->resource_manager()); + module_->mutable_servers().push_back(std::move(server)); + } } void ModuleService::add_model_from_registry_inlock_(API api, diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 70ef374fa..968a108ba 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -9,16 +9,7 @@ namespace viam { namespace sdk { Server::Server() : builder_(std::make_unique()) { - std::cout << "creating a new server! this should only happen once!" << std::endl; auto new_manager = std::make_shared(); - // CR erodkin: is this the best place to do this? Probably it makes more sense when we actually - // register the service. see if we can figure out how to do that. Related: if we can do it right - // then we don't need to manage resources separately in the module manager probably? it'll also - // have a server and that server can track everything for them. But we'll need to test that! - // - // CR erodkin: is it possible we have a duplicate in the registered resources somehow and that's - // what is causing the issue? Look into that possibility and/or the above CR as a separate - // implementation for (const auto& rr : Registry::registered_resources()) { auto server = rr.second->create_resource_server(new_manager, *this); managed_servers_.emplace(server->api(), std::move(server)); @@ -29,21 +20,20 @@ Server::~Server() { shutdown(); } +std::shared_ptr Server::lookup_resource_server(API& api) { + if (managed_servers_.find(api) == managed_servers_.end()) { + return nullptr; + } + + return managed_servers_.at(api); +} + void Server::register_service(grpc::Service* service) { if (!builder_) { throw std::runtime_error("Cannot register a new service after the server has started"); } - std::cout << "registering a service! " << std::endl; - - try { - builder_->RegisterService(service); - } catch (const std::runtime_error& e) { - std::cout << "caught an error!\n" << std::flush; - BOOST_LOG_TRIVIAL(warning) << "Failed to register service: " << e.what(); - } catch (...) { - std::cout << "caught an error!\n" << std::flush; - BOOST_LOG_TRIVIAL(warning) << "Failed to register service with unknown error"; - } + + builder_->RegisterService(service); } void Server::add_resource(std::shared_ptr resource) { diff --git a/src/viam/sdk/rpc/server.hpp b/src/viam/sdk/rpc/server.hpp index 540668c49..3eaf5ead9 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -39,8 +39,15 @@ class Server { /// @throws `std::runtime_error` if called after the server has been `start`ed. void register_service(grpc::Service* service); + /// // CR erodkin: is this necessary? it doesn't feel like it should be but let's see! + /// @brief Returns reference to managed resource server. + /// @param api The api of the managed resource server. + /// @returns The requested resource server, or nullptr if it doesn't exist. + std::shared_ptr lookup_resource_server(API& api); + // CR erodkin: I don't love having this on server. Consider if we can put this somewhere - // else? + // else? But that means server not holding onto the ResourceServers either. Something to + // consider! /// @brief Adds a specific managed resource to the associated resource server /// @param resource The resource to add /// @throws `std::runtime_error` if a matching `ResourceServer` doesn't exist in the server, From e084c1ade6e894237e692c8b7a03d483547558dd Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 31 Jan 2024 11:21:15 -0500 Subject: [PATCH 08/20] cleanup --- src/viam/examples/modules/complex/client.cpp | 5 -- src/viam/sdk/module/module.cpp | 12 ----- src/viam/sdk/module/module.hpp | 6 --- src/viam/sdk/module/service.cpp | 49 +++++--------------- src/viam/sdk/module/service.hpp | 5 -- src/viam/sdk/registry/registry.cpp | 2 - src/viam/sdk/rpc/server.cpp | 6 +-- src/viam/sdk/rpc/server.hpp | 9 +--- src/viam/sdk/tests/test_utils.hpp | 23 ++------- 9 files changed, 18 insertions(+), 99 deletions(-) diff --git a/src/viam/examples/modules/complex/client.cpp b/src/viam/examples/modules/complex/client.cpp index 4f6adffb5..292154175 100644 --- a/src/viam/examples/modules/complex/client.cpp +++ b/src/viam/examples/modules/complex/client.cpp @@ -42,16 +42,12 @@ int main() { // Register custom gizmo and summation API so robot client can access resources // of that type from the server. Registry::register_resource(API::get()); - // Registry::register_resource(API::get(), Gizmo::resource_registration()); Registry::register_resource( API::get()); - // Registry::register_resource(API::get(), Summation::resource_registration()); - std::cout << "registered resources\n" << std::flush; // Connect to robot. std::shared_ptr robot = RobotClient::at_address(address, options); // Print resources. - std::cout << "Resources" << std::endl; std::vector* resource_names = robot->resource_names(); for (const ResourceName& resource : *resource_names) { std::cout << "\tname: " << resource.name() << " (type:" << resource.type() @@ -65,7 +61,6 @@ int main() { return EXIT_FAILURE; } - std::cout << "trying to do a thing " << std::endl; bool do_one_ret = gc->do_one("arg1"); std::cout << "gizmo1 do_one returned: " << do_one_ret << std::endl; bool do_one_client_stream_ret = gc->do_one_client_stream({"arg1", "arg1", "arg1"}); diff --git a/src/viam/sdk/module/module.cpp b/src/viam/sdk/module/module.cpp index 5ba81bfee..87a3ee908 100644 --- a/src/viam/sdk/module/module.cpp +++ b/src/viam/sdk/module/module.cpp @@ -34,18 +34,6 @@ HandlerMap_& Module::mutable_handles() { const std::shared_ptr& Module::channel() const { return channel_; }; -const std::unordered_map>& Module::services() const { - return services_; -}; -std::unordered_map>& Module::mutable_services() { - return services_; -}; -const std::vector>& Module::servers() const { - return servers_; -}; -std::vector>& Module::mutable_servers() { - return servers_; -}; } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/module/module.hpp b/src/viam/sdk/module/module.hpp index 440cd8c2b..18c7a0b82 100644 --- a/src/viam/sdk/module/module.hpp +++ b/src/viam/sdk/module/module.hpp @@ -19,10 +19,6 @@ class Module { const HandlerMap_& handles() const; HandlerMap_& mutable_handles(); const std::shared_ptr& channel() const; - const std::unordered_map>& services() const; - std::unordered_map>& mutable_services(); - const std::vector>& servers() const; - std::vector>& mutable_servers(); private: std::string name_; @@ -30,8 +26,6 @@ class Module { bool ready_; HandlerMap_ handles_; std::shared_ptr channel_; - std::unordered_map> services_; - std::vector> servers_; }; } // namespace sdk diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 4fb625ea6..1171c6612 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -69,6 +69,7 @@ std::shared_ptr ModuleService::get_parent_resource_(Name name) { return parent_->resource_by_name(name.to_proto()); } +// TODO(RSDK-6528) - to the extent possible, switch to using `server_helper` ::grpc::Status ModuleService::AddResource(::grpc::ServerContext* context, const ::viam::module::v1::AddResourceRequest* request, ::viam::module::v1::AddResourceResponse* response) { @@ -86,14 +87,12 @@ ::grpc::Status ModuleService::AddResource(::grpc::ServerContext* context, return grpc::Status(::grpc::INTERNAL, exc.what()); } }; - const std::unordered_map>& services = module_->services(); - if (services.find(cfg.api()) == services.end()) { - return grpc::Status(grpc::UNKNOWN, "module cannot service api " + cfg.api().to_string()); + try { + server_->add_resource(res); + } catch (const std::exception& exc) { + return grpc::Status(::grpc::INTERNAL, exc.what()); } - const std::shared_ptr manager = services.at(cfg.api()); - manager->add(cfg.resource_name(), res); - return grpc::Status(); }; @@ -106,11 +105,11 @@ ::grpc::Status ModuleService::ReconfigureResource( const Dependencies deps = get_dependencies_(request->dependencies(), cfg.name()); - const std::unordered_map>& services = module_->services(); - if (services.find(cfg.api()) == services.end()) { + auto resource_server = server_->lookup_resource_server(cfg.api()); + if (!resource_server) { return grpc::Status(grpc::UNKNOWN, "no rpc service for config: " + cfg.api().to_string()); } - const std::shared_ptr manager = services.at(cfg.api()); + auto manager = resource_server->resource_manager(); // see if our resource is reconfigurable. if it is, reconfigure const std::shared_ptr res = manager->resource(cfg.resource_name().name()); @@ -176,11 +175,11 @@ ::grpc::Status ModuleService::RemoveResource( const ::viam::module::v1::RemoveResourceRequest* request, ::viam::module::v1::RemoveResourceResponse* response) { auto name = Name::from_string(request->name()); - const std::unordered_map>& services = module_->services(); - if (services.find(name.api()) == services.end()) { + auto resource_server = server_->lookup_resource_server(name.api()); + if (!resource_server) { return grpc::Status(grpc::UNKNOWN, "no grpc service for " + name.api().to_string()); } - const std::shared_ptr manager = services.at(name.api()); + const std::shared_ptr manager = resource_server->resource_manager(); const std::shared_ptr res = manager->resource(name.name()); if (!res) { return grpc::Status( @@ -263,30 +262,9 @@ ModuleService::~ModuleService() { } } -void ModuleService::add_api_from_registry_inlock_(API api, const std::lock_guard&) { - const std::unordered_map>& services = module_->services(); - if (services.find(api) != services.end()) { - return; - } - - const std::shared_ptr rs = Registry::lookup_resource(api); - auto server = server_->lookup_resource_server(api); - // CR erodkin: instead of having module hold on to copies of the services/servers, just - // point to what's in the server when doing add operations - if (server) { - module_->mutable_services().emplace(api, server->resource_manager()); - module_->mutable_servers().push_back(std::move(server)); - } -} - void ModuleService::add_model_from_registry_inlock_(API api, Model model, const std::lock_guard& lock) { - const std::unordered_map>& services = module_->services(); - if (services.find(api) == services.end()) { - add_api_from_registry_inlock_(api, lock); - } - const std::shared_ptr creator = Registry::lookup_resource(api); std::string name; const google::protobuf::ServiceDescriptor* sd = nullptr; @@ -298,11 +276,6 @@ void ModuleService::add_model_from_registry_inlock_(API api, module_->mutable_handles().add_model(model, rpc_subtype); }; -void ModuleService::add_api_from_registry(API api) { - const std::lock_guard lock(lock_); - return add_api_from_registry_inlock_(api, lock); -} - void ModuleService::add_model_from_registry(API api, Model model) { const std::lock_guard lock(lock_); return add_model_from_registry_inlock_(api, model, lock); diff --git a/src/viam/sdk/module/service.hpp b/src/viam/sdk/module/service.hpp index 5b2b74511..077bb4616 100644 --- a/src/viam/sdk/module/service.hpp +++ b/src/viam/sdk/module/service.hpp @@ -42,10 +42,6 @@ class ModuleService : viam::module::v1::ModuleService::Service { /// (this happens when the RDK shuts down). void serve(); - /// @brief Adds an API to the module that has already been registered. - /// @param api The API to add. - void add_api_from_registry(API api); - /// @brief Adds an API/model pair to the module; both the API and model should have /// already been registered. If the ModuleService was constructed with a vector /// of ModelRegistration, the passed in models will already be registered and added. @@ -76,7 +72,6 @@ class ModuleService : viam::module::v1::ModuleService::Service { ::viam::module::v1::ValidateConfigResponse* response) override; void add_model_from_registry_inlock_(API api, Model model, const std::lock_guard&); - void add_api_from_registry_inlock_(API api, const std::lock_guard& lock); Dependencies get_dependencies_(google::protobuf::RepeatedPtrField const& proto, std::string const& resource_name); std::shared_ptr get_parent_resource_(Name name); diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 6d4b16623..4d35cd719 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -157,10 +157,8 @@ const google::protobuf::ServiceDescriptor* ResourceRegistration::service_descrip void register_resources() { // Register all components - std::cout << "registering base!!" << std::endl; Registry::register_resource( API::get()); - std::cout << "registered base!!" << std::endl; Registry::register_resource( API::get()); Registry::register_resource( diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 968a108ba..15559e6a8 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -20,7 +20,7 @@ Server::~Server() { shutdown(); } -std::shared_ptr Server::lookup_resource_server(API& api) { +std::shared_ptr Server::lookup_resource_server(const API& api) { if (managed_servers_.find(api) == managed_servers_.end()) { return nullptr; } @@ -37,10 +37,6 @@ void Server::register_service(grpc::Service* service) { } void Server::add_resource(std::shared_ptr resource) { - if (!builder_) { - throw std::runtime_error("Cannot add a new resource after the server has started"); - } - auto api = resource->api(); if (managed_servers_.find(api) == managed_servers_.end()) { std::ostringstream buffer; diff --git a/src/viam/sdk/rpc/server.hpp b/src/viam/sdk/rpc/server.hpp index 3eaf5ead9..d33b81669 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -39,19 +39,14 @@ class Server { /// @throws `std::runtime_error` if called after the server has been `start`ed. void register_service(grpc::Service* service); - /// // CR erodkin: is this necessary? it doesn't feel like it should be but let's see! /// @brief Returns reference to managed resource server. /// @param api The api of the managed resource server. /// @returns The requested resource server, or nullptr if it doesn't exist. - std::shared_ptr lookup_resource_server(API& api); + std::shared_ptr lookup_resource_server(const API& api); - // CR erodkin: I don't love having this on server. Consider if we can put this somewhere - // else? But that means server not holding onto the ResourceServers either. Something to - // consider! /// @brief Adds a specific managed resource to the associated resource server /// @param resource The resource to add - /// @throws `std::runtime_error` if a matching `ResourceServer` doesn't exist in the server, - /// or if called after the server has `start`ed. + /// @throws `std::runtime_error` if a matching `ResourceServer` doesn't exist in the server. void add_resource(std::shared_ptr resource); /// @brief Adds a listening port to the server. diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index 4724bbe48..8d1286834 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -49,27 +49,12 @@ class TestServer { // The passed in test_case function will have access to the created ResourceClient. template void client_to_mock_pipeline(std::shared_ptr mock, F&& test_case) { - if (!Registry::is_initialized()) { - Registry::initialize(); - } - // Create a ResourceManager. Add the mock resource to the ResourceManager. - // Create a Server. Use the mock's API to create a resource-specific - // server (like MotorServer) from the ResourceManager and Server. Start the - // Server. - // - // CR erodkin: commented code should be deleted BUT we might want to go back to this kind - // of pattern if we do JIT server registration (as opposed to registering all resource - // servers when we start the RPC server, as we do now). So keeping it around just in case. - // - // auto rm = std::make_shared(); - // rm->add(mock->name(), mock); + Registry::initialize(); auto server = std::make_shared(); + + // normally the high level server service (either robot or module) handles adding managed + // resources, but in this case we must do it ourselves. server->add_resource(mock); - auto rs = sdk::Registry::lookup_resource(mock->api()); - // resource_server is unused; we call create_resource_server to call - // register_service and associate the Server with the resource-specific - // server (like MotorServer). - // auto resource_server = rs->create_resource_server(rm, *server); server->start(); // Create a resource-specific client to the mock over an established From bb9b55ff81f8ab0d117a0726711a597a8b9b570a Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 31 Jan 2024 11:42:02 -0500 Subject: [PATCH 09/20] clang tidy complaint --- 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 53eaac254..04e9260c4 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -28,7 +28,7 @@ std::vector resource_names_for_resource(const std::shared_ptr resource_names; - for (auto& kv : Registry::registered_models()) { + for (const auto& kv : Registry::registered_models()) { const std::shared_ptr reg = kv.second; if (reg->api().to_string() == resource->api().to_string()) { resource_type = reg->api().resource_type(); From fbccc83e25633df5567046a03d77276bd93bea50 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 31 Jan 2024 11:59:26 -0500 Subject: [PATCH 10/20] delete notes to self, import grpcpp channel in mlmodelclient.hpp --- src/viam/examples/modules/complex/main.cpp | 1 - src/viam/sdk/robot/service.hpp | 1 - src/viam/sdk/rpc/server.hpp | 2 -- src/viam/sdk/services/mlmodel/client.hpp | 2 ++ 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/viam/examples/modules/complex/main.cpp b/src/viam/examples/modules/complex/main.cpp index ce9d1219f..8ce22ae03 100644 --- a/src/viam/examples/modules/complex/main.cpp +++ b/src/viam/examples/modules/complex/main.cpp @@ -34,7 +34,6 @@ int main(int argc, char** argv) { API summation_api = API::get(); Model mybase_model("viam", "base", "mybase"); - // // CR erodkin: add note about how we've reduced the comment here. // Make sure to explicity register resources with custom APIs. Registry::register_resource(gizmo_api); Registry::register_resource(summation_api); diff --git a/src/viam/sdk/robot/service.hpp b/src/viam/sdk/robot/service.hpp index 10fa6729c..902c54271 100644 --- a/src/viam/sdk/robot/service.hpp +++ b/src/viam/sdk/robot/service.hpp @@ -49,7 +49,6 @@ class RobotService_ : public ResourceServer, public viam::robot::v1::RobotServic ::viam::robot::v1::StopAllResponse* response) override; private: - // CR erodkin: this is a bit of a hacky workaround, leave a comment about it API api() const override; std::mutex lock_; std::vector generate_metadata(); diff --git a/src/viam/sdk/rpc/server.hpp b/src/viam/sdk/rpc/server.hpp index d33b81669..5e1a12f03 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -32,8 +32,6 @@ class Server { /// repeated calls. void start(); - // CR erodkin: deleted a todo here because it wouldn't play nice with user-defined modular - // resources. Leave a flyby comment to that effect in the PR. /// @brief Registers a gRPC service. /// @param service The gRPC service to be registered. /// @throws `std::runtime_error` if called after the server has been `start`ed. diff --git a/src/viam/sdk/services/mlmodel/client.hpp b/src/viam/sdk/services/mlmodel/client.hpp index 83b42bb86..437b37fd7 100644 --- a/src/viam/sdk/services/mlmodel/client.hpp +++ b/src/viam/sdk/services/mlmodel/client.hpp @@ -16,6 +16,8 @@ #include +#include + #include namespace viam { From 44a965b0d918c0bec1a59eb67a6d2badcb5cba5c Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 31 Jan 2024 12:28:50 -0500 Subject: [PATCH 11/20] some more cleanup and clang tidy stuff --- src/viam/examples/camera/example_camera.cpp | 4 ---- src/viam/examples/dial/example_dial.cpp | 4 ---- .../examples/dial_api_key/example_dial_api_key.cpp | 10 +++------- src/viam/examples/modules/complex/client.cpp | 10 ++++------ src/viam/examples/modules/complex/main.cpp | 4 +++- .../examples/modules/complex/test_complex_module.cpp | 2 -- src/viam/examples/motor/example_motor.cpp | 4 ---- src/viam/sdk/common/viam.cpp | 1 - src/viam/sdk/common/viam.hpp | 1 - src/viam/sdk/registry/registry.cpp | 4 ---- src/viam/sdk/registry/registry.hpp | 3 --- src/viam/sdk/rpc/server.cpp | 2 ++ src/viam/sdk/tests/test_robot.cpp | 4 +--- 13 files changed, 13 insertions(+), 40 deletions(-) delete mode 100644 src/viam/sdk/common/viam.cpp delete mode 100644 src/viam/sdk/common/viam.hpp diff --git a/src/viam/examples/camera/example_camera.cpp b/src/viam/examples/camera/example_camera.cpp index de45452bb..d32e56e72 100644 --- a/src/viam/examples/camera/example_camera.cpp +++ b/src/viam/examples/camera/example_camera.cpp @@ -15,10 +15,6 @@ int main() { using std::endl; namespace vs = ::viam::sdk; try { - // initialize the Registry to ensure all built-in components and gRPC server reflection - // are supported. - vs::Registry::initialize(); - // If you want to connect to a remote robot, this should be the url of the robot // Ex: xxx.xxx.viam.cloud std::string robot_address("localhost:8080"); diff --git a/src/viam/examples/dial/example_dial.cpp b/src/viam/examples/dial/example_dial.cpp index ed6b95c26..84770ad0a 100644 --- a/src/viam/examples/dial/example_dial.cpp +++ b/src/viam/examples/dial/example_dial.cpp @@ -36,10 +36,6 @@ int main() { std::string address(uri); Options options(1, opts); - // initialize the Registry to ensure all built-in components and gRPC server reflection - // are supported. - Registry::initialize(); - // connect to robot, ensure we can refresh it std::shared_ptr robot = RobotClient::at_address(address, options); diff --git a/src/viam/examples/dial_api_key/example_dial_api_key.cpp b/src/viam/examples/dial_api_key/example_dial_api_key.cpp index 2ba11ad80..d49acd117 100644 --- a/src/viam/examples/dial_api_key/example_dial_api_key.cpp +++ b/src/viam/examples/dial_api_key/example_dial_api_key.cpp @@ -26,11 +26,11 @@ using viam::robot::v1::Status; using namespace viam::sdk; int main() { - const char* uri = "webrtc-test-main.jkek76kqnh.viam.cloud"; + const char* uri = ""; DialOptions dial_options; std::string type = "api-key"; - std::string entity = "a9dcf212-3397-4318-bb1e-f5c36b3cafc1"; - std::string payload = "2ml60ys1j4i9v0pecpxahe654yxpc1dz"; + std::string entity = ""; + std::string payload = ""; dial_options.set_entity(entity); Credentials credentials(type, payload); dial_options.set_credentials(credentials); @@ -38,10 +38,6 @@ int main() { std::string address(uri); Options options(1, opts); - // initialize the Registry to ensure all built-in components and gRPC server reflection - // are supported. - Registry::initialize(); - // connect to robot, ensure we can refresh it std::shared_ptr robot = RobotClient::at_address(address, options); diff --git a/src/viam/examples/modules/complex/client.cpp b/src/viam/examples/modules/complex/client.cpp index 292154175..55d8fd58a 100644 --- a/src/viam/examples/modules/complex/client.cpp +++ b/src/viam/examples/modules/complex/client.cpp @@ -27,12 +27,10 @@ using namespace viam::sdk; int main() { - const char* uri = "webrtc-test-main.jkek76kqnh.viam.cloud"; + const char* uri = ""; DialOptions dial_options; - std::string type = "api-key"; - std::string entity = "a9dcf212-3397-4318-bb1e-f5c36b3cafc1"; - std::string payload = "2ml60ys1j4i9v0pecpxahe654yxpc1dz"; - dial_options.set_entity(entity); + std::string type = ""; + std::string payload = ""; Credentials credentials(type, payload); dial_options.set_credentials(credentials); boost::optional opts(dial_options); @@ -48,6 +46,7 @@ int main() { // Connect to robot. std::shared_ptr robot = RobotClient::at_address(address, options); // Print resources. + std::cout << "Resources" << std::endl; std::vector* resource_names = robot->resource_names(); for (const ResourceName& resource : *resource_names) { std::cout << "\tname: " << resource.name() << " (type:" << resource.type() @@ -60,7 +59,6 @@ int main() { std::cerr << "could not get 'gizmo1' resource from robot" << std::endl; return EXIT_FAILURE; } - bool do_one_ret = gc->do_one("arg1"); std::cout << "gizmo1 do_one returned: " << do_one_ret << std::endl; bool do_one_client_stream_ret = gc->do_one_client_stream({"arg1", "arg1", "arg1"}); diff --git a/src/viam/examples/modules/complex/main.cpp b/src/viam/examples/modules/complex/main.cpp index 8ce22ae03..e93624e30 100644 --- a/src/viam/examples/modules/complex/main.cpp +++ b/src/viam/examples/modules/complex/main.cpp @@ -28,7 +28,10 @@ using namespace viam::sdk; int main(int argc, char** argv) { + // Be sure to initialize the registry so that all built-in resources types and server reflection + // are supported. Registry::initialize(); + API base_api = API::get(); API gizmo_api = API::get(); API summation_api = API::get(); @@ -45,7 +48,6 @@ int main(int argc, char** argv) { MyBase::validate); Model mygizmo_model("viam", "gizmo", "mygizmo"); - // Make sure to explicity register resources with custom APIs. std::shared_ptr mygizmo_mr = std::make_shared( gizmo_api, mygizmo_model, diff --git a/src/viam/examples/modules/complex/test_complex_module.cpp b/src/viam/examples/modules/complex/test_complex_module.cpp index 4e9ca0b5c..0b90d49a2 100644 --- a/src/viam/examples/modules/complex/test_complex_module.cpp +++ b/src/viam/examples/modules/complex/test_complex_module.cpp @@ -27,10 +27,8 @@ using namespace viam::sdktests; struct RegisterGizmoAndSummationFixture { RegisterGizmoAndSummationFixture() { Registry::register_resource(API::get()); - // Registry::register_resource(API::get(), Gizmo::resource_registration()); Registry::register_resource( API::get()); - // Registry::register_resource(API::get(), Summation::resource_registration()); } // Test teardown is a noop; diff --git a/src/viam/examples/motor/example_motor.cpp b/src/viam/examples/motor/example_motor.cpp index 339c96a6d..60b78ef2c 100644 --- a/src/viam/examples/motor/example_motor.cpp +++ b/src/viam/examples/motor/example_motor.cpp @@ -27,10 +27,6 @@ int main() { namespace vs = ::viam::sdk; try { - // initialize the Registry to ensure all built-in components and gRPC server reflection - // are supported. - vs::Registry::initialize(); - // If you want to connect to a remote robot, this should be the url of the robot // Ex: xxx.xxx.viam.cloud std::string robot_address("localhost:8080"); diff --git a/src/viam/sdk/common/viam.cpp b/src/viam/sdk/common/viam.cpp deleted file mode 100644 index 8b1378917..000000000 --- a/src/viam/sdk/common/viam.cpp +++ /dev/null @@ -1 +0,0 @@ - diff --git a/src/viam/sdk/common/viam.hpp b/src/viam/sdk/common/viam.hpp deleted file mode 100644 index 8b1378917..000000000 --- a/src/viam/sdk/common/viam.hpp +++ /dev/null @@ -1 +0,0 @@ - diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 4d35cd719..3a6d6c976 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -196,10 +196,6 @@ void register_resources() { API::get()); } -bool Registry::is_initialized() { - return initialized_; -} - void Registry::initialize() { if (initialized_) { BOOST_LOG_TRIVIAL(warning) diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index 017c85d51..bd640463c 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -187,9 +187,6 @@ class Registry { /// @brief Initialized the Viam registry. No-op if it has already been called. static void initialize(); - /// @brief Returns whether or not the Registry has been `initialize`d. - static bool is_initialized(); - private: static bool initialized_; diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 15559e6a8..5dae1d1be 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -1,5 +1,7 @@ #include +#include + #include #include diff --git a/src/viam/sdk/tests/test_robot.cpp b/src/viam/sdk/tests/test_robot.cpp index 9ee1f13b2..e6c0f338c 100644 --- a/src/viam/sdk/tests/test_robot.cpp +++ b/src/viam/sdk/tests/test_robot.cpp @@ -38,9 +38,7 @@ BOOST_AUTO_TEST_SUITE(test_robot) // the robot client and the mock robot service. template void robot_client_to_mocks_pipeline(F&& test_case) { - if (!Registry::is_initialized()) { - Registry::initialize(); - } + Registry::initialize(); // Create a ResourceManager. Add a few mock resources to the // ResourceManager. Create a Server. Create a MockRobotService from the // ResourceManager and Server. Start the Server. From 1902a0ffc6661125c79c5bf548cf497093faf237 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 31 Jan 2024 12:51:24 -0500 Subject: [PATCH 12/20] const stuff for clang tidy --- src/viam/sdk/module/service.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 1171c6612..6a8fa6b6f 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -74,7 +74,7 @@ ::grpc::Status ModuleService::AddResource(::grpc::ServerContext* context, const ::viam::module::v1::AddResourceRequest* request, ::viam::module::v1::AddResourceResponse* response) { const viam::app::v1::ComponentConfig& proto = request->config(); - ResourceConfig cfg = ResourceConfig::from_proto(proto); + const ResourceConfig cfg = ResourceConfig::from_proto(proto); const std::lock_guard lock(lock_); std::shared_ptr res; From 5b5530c6bddf7554cb00e7747ace27f94f4db8d3 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Wed, 31 Jan 2024 14:44:11 -0500 Subject: [PATCH 13/20] registry initialize when starting server --- src/viam/examples/modules/complex/main.cpp | 4 ---- src/viam/sdk/registry/registry.hpp | 8 -------- src/viam/sdk/rpc/server.cpp | 3 ++- src/viam/sdk/tests/test_robot.cpp | 1 - src/viam/sdk/tests/test_utils.hpp | 1 - 5 files changed, 2 insertions(+), 15 deletions(-) diff --git a/src/viam/examples/modules/complex/main.cpp b/src/viam/examples/modules/complex/main.cpp index e93624e30..4d6558ca3 100644 --- a/src/viam/examples/modules/complex/main.cpp +++ b/src/viam/examples/modules/complex/main.cpp @@ -28,10 +28,6 @@ using namespace viam::sdk; int main(int argc, char** argv) { - // Be sure to initialize the registry so that all built-in resources types and server reflection - // are supported. - Registry::initialize(); - API base_api = API::get(); API gizmo_api = API::get(); API summation_api = API::get(); diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index bd640463c..b13572d8a 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -117,25 +117,21 @@ class Registry { /// @brief Registers a resource with the Registry. /// @param resource An object containing resource registration information. /// @throws `std::runtime_error` if the resource has already been registered. - /// @throws `std::runtime_error` if `initialize` has not been called. static void register_model(std::shared_ptr resource); /// @brief Lookup a given registered resource. /// @param name The name of the resource to lookup. /// @return a `shared_ptr` to the resource's registration data. - /// @throws `std::runtime_error` if `initialize` has not been called. static std::shared_ptr lookup_model(std::string name); /// @brief Lookup a given registered resource. /// @param api The api of the resource to lookup. /// @param model The model of the resource to lookup. /// @return a `shared_ptr` to the resource's registration data. - /// @throws `std::runtime_error` if `initialize` has not been called. static std::shared_ptr lookup_model(API api, Model model); /// @brief Register an api. /// @param api The api to be registered. - /// @throws `std::runtime_error` if `initialize` has not been called. template static void register_resource(API api) { class ResourceRegistration2 final : public ResourceRegistration { @@ -162,25 +158,21 @@ class Registry { /// @brief Register an api. /// @param api The api to be registered. /// @param resource_registration `ResourceRegistration` with resource functionality. - /// @throws `std::runtime_error` if `initialize` has not been called. static void register_resource(API api, std::shared_ptr resource_registration); /// @brief Lookup a registered api. /// @param api The api to lookup. /// @return A `shared_ptr` to the registered api's `ResourceRegistration`. - /// @throws `std::runtime_error` if `initialize` has not been called. static std::shared_ptr lookup_resource(API api); /// @brief Provide information on registered resource models. /// @return A map from name to `ModelRegistration` of all registered resource models. - /// @throws `std::runtime_error` if `initialize` has not been called. static const std::unordered_map>& registered_models(); /// @brief Provide access to registered resources. /// @return A map from `API` to `ResourceRegistration` of all registered resources. - /// @throws `std::runtime_error` if `initialize` has not been called. static const std::unordered_map>& registered_resources(); diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 5dae1d1be..b050e41e0 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -11,8 +11,9 @@ namespace viam { namespace sdk { Server::Server() : builder_(std::make_unique()) { - auto new_manager = std::make_shared(); + Registry::initialize(); for (const auto& rr : Registry::registered_resources()) { + auto new_manager = std::make_shared(); auto server = rr.second->create_resource_server(new_manager, *this); managed_servers_.emplace(server->api(), std::move(server)); } diff --git a/src/viam/sdk/tests/test_robot.cpp b/src/viam/sdk/tests/test_robot.cpp index e6c0f338c..2f047acae 100644 --- a/src/viam/sdk/tests/test_robot.cpp +++ b/src/viam/sdk/tests/test_robot.cpp @@ -38,7 +38,6 @@ BOOST_AUTO_TEST_SUITE(test_robot) // the robot client and the mock robot service. template void robot_client_to_mocks_pipeline(F&& test_case) { - Registry::initialize(); // Create a ResourceManager. Add a few mock resources to the // ResourceManager. Create a Server. Create a MockRobotService from the // ResourceManager and Server. Start the Server. diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index 8d1286834..a6b04fc3f 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -49,7 +49,6 @@ class TestServer { // The passed in test_case function will have access to the created ResourceClient. template void client_to_mock_pipeline(std::shared_ptr mock, F&& test_case) { - Registry::initialize(); auto server = std::make_shared(); // normally the high level server service (either robot or module) handles adding managed From a5eace363d2fd438b12f77922f7df26c66f97603 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Thu, 8 Feb 2024 08:48:51 -0500 Subject: [PATCH 14/20] remove api method from servers --- src/viam/examples/modules/complex/gizmo/api.cpp | 4 ---- src/viam/examples/modules/complex/gizmo/api.hpp | 1 - src/viam/examples/modules/complex/proto/buf.lock | 4 ++-- src/viam/examples/modules/complex/summation/api.cpp | 4 ---- src/viam/examples/modules/complex/summation/api.hpp | 1 - src/viam/sdk/components/base/server.cpp | 4 ---- src/viam/sdk/components/base/server.hpp | 1 - src/viam/sdk/components/board/server.cpp | 4 ---- src/viam/sdk/components/board/server.hpp | 1 - src/viam/sdk/components/camera/server.cpp | 4 ---- src/viam/sdk/components/camera/server.hpp | 1 - src/viam/sdk/components/encoder/server.cpp | 4 ---- src/viam/sdk/components/encoder/server.hpp | 1 - src/viam/sdk/components/generic/server.cpp | 4 ---- src/viam/sdk/components/generic/server.hpp | 1 - src/viam/sdk/components/motor/server.cpp | 4 ---- src/viam/sdk/components/motor/server.hpp | 1 - src/viam/sdk/components/movement_sensor/server.cpp | 4 ---- src/viam/sdk/components/movement_sensor/server.hpp | 1 - src/viam/sdk/components/power_sensor/server.cpp | 4 ---- src/viam/sdk/components/power_sensor/server.hpp | 1 - src/viam/sdk/components/sensor/server.cpp | 4 ---- src/viam/sdk/components/sensor/server.hpp | 1 - src/viam/sdk/components/servo/server.cpp | 4 ---- src/viam/sdk/components/servo/server.hpp | 1 - src/viam/sdk/resource/resource_server_base.cpp | 4 ---- src/viam/sdk/resource/resource_server_base.hpp | 2 -- src/viam/sdk/robot/service.cpp | 4 ---- src/viam/sdk/robot/service.hpp | 1 - src/viam/sdk/rpc/server.cpp | 2 +- src/viam/sdk/services/generic/server.cpp | 4 ---- src/viam/sdk/services/generic/server.hpp | 1 - src/viam/sdk/services/mlmodel/server.cpp | 4 ---- src/viam/sdk/services/mlmodel/server.hpp | 1 - src/viam/sdk/services/motion/server.cpp | 4 ---- src/viam/sdk/services/motion/server.hpp | 1 - 36 files changed, 3 insertions(+), 89 deletions(-) diff --git a/src/viam/examples/modules/complex/gizmo/api.cpp b/src/viam/examples/modules/complex/gizmo/api.cpp index 733a7e8d8..4059c9034 100644 --- a/src/viam/examples/modules/complex/gizmo/api.cpp +++ b/src/viam/examples/modules/complex/gizmo/api.cpp @@ -16,10 +16,6 @@ using namespace viam::sdk; using namespace viam::component::gizmo::v1; -API GizmoServer::api() const { - return API::get(); -} - API Gizmo::api() const { return API::get(); } diff --git a/src/viam/examples/modules/complex/gizmo/api.hpp b/src/viam/examples/modules/complex/gizmo/api.hpp index 834c5b3ec..a9b5fcb68 100644 --- a/src/viam/examples/modules/complex/gizmo/api.hpp +++ b/src/viam/examples/modules/complex/gizmo/api.hpp @@ -58,7 +58,6 @@ class GizmoClient : public Gizmo { class GizmoServer : public ResourceServer, public GizmoService::Service { public: explicit GizmoServer(std::shared_ptr manager); - API api() const override; grpc::Status DoOne(grpc::ServerContext* context, const DoOneRequest* request, diff --git a/src/viam/examples/modules/complex/proto/buf.lock b/src/viam/examples/modules/complex/proto/buf.lock index 4144ecf50..013f46cac 100644 --- a/src/viam/examples/modules/complex/proto/buf.lock +++ b/src/viam/examples/modules/complex/proto/buf.lock @@ -4,5 +4,5 @@ deps: - remote: buf.build owner: googleapis repository: googleapis - commit: a86849a25cc04f4dbe9b15ddddfbc488 - digest: shake256:e19143328f8cbfe13fc226aeee5e63773ca494693a72740a7560664270039a380d94a1344234b88c7691311460df9a9b1c2982190d0a2612eae80368718e1943 + commit: e874a0be2bf140a5a4c7d4122c635823 + digest: shake256:4fe3036b4d706f6ee2b13c730bd04777f021dfd02ed27e6e40480acfe664a7548238312ee0727fd77648a38d227e296d43f4a38a34cdd46068156211016d9657 diff --git a/src/viam/examples/modules/complex/summation/api.cpp b/src/viam/examples/modules/complex/summation/api.cpp index b9f3dc6d1..cab868ad8 100644 --- a/src/viam/examples/modules/complex/summation/api.cpp +++ b/src/viam/examples/modules/complex/summation/api.cpp @@ -20,10 +20,6 @@ API Summation::api() const { return API::get(); } -API SummationServer::api() const { - return API::get(); -} - API API::traits::api() { return {"viam", "service", "summation"}; } diff --git a/src/viam/examples/modules/complex/summation/api.hpp b/src/viam/examples/modules/complex/summation/api.hpp index 1118732fb..2f253525b 100644 --- a/src/viam/examples/modules/complex/summation/api.hpp +++ b/src/viam/examples/modules/complex/summation/api.hpp @@ -55,7 +55,6 @@ class SummationClient : public Summation { class SummationServer : public ResourceServer, public SummationService::Service { public: explicit SummationServer(std::shared_ptr manager); - API api() const override; grpc::Status Sum(grpc::ServerContext* context, const SumRequest* request, diff --git a/src/viam/sdk/components/base/server.cpp b/src/viam/sdk/components/base/server.cpp index 3a3b7260d..b04080402 100644 --- a/src/viam/sdk/components/base/server.cpp +++ b/src/viam/sdk/components/base/server.cpp @@ -112,9 +112,5 @@ ::grpc::Status BaseServer::DoCommand(grpc::ServerContext* context, }); } -API BaseServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/base/server.hpp b/src/viam/sdk/components/base/server.hpp index 483d35744..6755979ec 100644 --- a/src/viam/sdk/components/base/server.hpp +++ b/src/viam/sdk/components/base/server.hpp @@ -19,7 +19,6 @@ namespace sdk { class BaseServer : public ResourceServer, public viam::component::base::v1::BaseService::Service { public: explicit BaseServer(std::shared_ptr manager); - API api() const override; ::grpc::Status MoveStraight( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/board/server.cpp b/src/viam/sdk/components/board/server.cpp index d5504f6c8..5a218e244 100644 --- a/src/viam/sdk/components/board/server.cpp +++ b/src/viam/sdk/components/board/server.cpp @@ -204,9 +204,5 @@ ::grpc::Status BoardServer::GetGeometries( }); } -API BoardServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/board/server.hpp b/src/viam/sdk/components/board/server.hpp index 361ac2e63..8bf5a6c39 100644 --- a/src/viam/sdk/components/board/server.hpp +++ b/src/viam/sdk/components/board/server.hpp @@ -19,7 +19,6 @@ class BoardServer : public ResourceServer, public viam::component::board::v1::BoardService::Service { public: explicit BoardServer(std::shared_ptr manager); - API api() const override; ::grpc::Status Status(::grpc::ServerContext* context, const ::viam::component::board::v1::StatusRequest* request, diff --git a/src/viam/sdk/components/camera/server.cpp b/src/viam/sdk/components/camera/server.cpp index d792bea51..f4a594a0f 100644 --- a/src/viam/sdk/components/camera/server.cpp +++ b/src/viam/sdk/components/camera/server.cpp @@ -116,9 +116,5 @@ ::grpc::Status CameraServer::GetProperties( }); } -API CameraServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/camera/server.hpp b/src/viam/sdk/components/camera/server.hpp index 703a9e411..a7ffdd3b2 100644 --- a/src/viam/sdk/components/camera/server.hpp +++ b/src/viam/sdk/components/camera/server.hpp @@ -19,7 +19,6 @@ class CameraServer : public ResourceServer, public viam::component::camera::v1::CameraService::Service { public: explicit CameraServer(std::shared_ptr manager); - API api() const override; ::grpc::Status DoCommand(::grpc::ServerContext* context, const ::viam::common::v1::DoCommandRequest* request, diff --git a/src/viam/sdk/components/encoder/server.cpp b/src/viam/sdk/components/encoder/server.cpp index de47ec0c5..981127012 100644 --- a/src/viam/sdk/components/encoder/server.cpp +++ b/src/viam/sdk/components/encoder/server.cpp @@ -68,9 +68,5 @@ ::grpc::Status EncoderServer::DoCommand(grpc::ServerContext* context, }); } -API EncoderServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/encoder/server.hpp b/src/viam/sdk/components/encoder/server.hpp index 0c9b9f378..c67c59e43 100644 --- a/src/viam/sdk/components/encoder/server.hpp +++ b/src/viam/sdk/components/encoder/server.hpp @@ -19,7 +19,6 @@ class EncoderServer : public ResourceServer, public viam::component::encoder::v1::EncoderService::Service { public: explicit EncoderServer(std::shared_ptr manager); - API api() const override; ::grpc::Status GetPosition( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/generic/server.cpp b/src/viam/sdk/components/generic/server.cpp index 1c580712d..95cfedeb7 100644 --- a/src/viam/sdk/components/generic/server.cpp +++ b/src/viam/sdk/components/generic/server.cpp @@ -33,9 +33,5 @@ ::grpc::Status GenericComponentServer::GetGeometries( }); } -API GenericComponentServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/generic/server.hpp b/src/viam/sdk/components/generic/server.hpp index 7ca0d1c6e..d1061b9c0 100644 --- a/src/viam/sdk/components/generic/server.hpp +++ b/src/viam/sdk/components/generic/server.hpp @@ -19,7 +19,6 @@ class GenericComponentServer : public ResourceServer, public viam::component::generic::v1::GenericService::Service { public: explicit GenericComponentServer(std::shared_ptr manager); - API api() const override; ::grpc::Status DoCommand(::grpc::ServerContext* context, const ::viam::common::v1::DoCommandRequest* request, diff --git a/src/viam/sdk/components/motor/server.cpp b/src/viam/sdk/components/motor/server.cpp index 5fbefe926..5ba2dbb89 100644 --- a/src/viam/sdk/components/motor/server.cpp +++ b/src/viam/sdk/components/motor/server.cpp @@ -126,9 +126,5 @@ ::grpc::Status MotorServer::DoCommand(grpc::ServerContext* context, }); } -API MotorServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/motor/server.hpp b/src/viam/sdk/components/motor/server.hpp index 47ef2088b..7e2a22c94 100644 --- a/src/viam/sdk/components/motor/server.hpp +++ b/src/viam/sdk/components/motor/server.hpp @@ -19,7 +19,6 @@ class MotorServer : public ResourceServer, public viam::component::motor::v1::MotorService::Service { public: explicit MotorServer(std::shared_ptr manager); - API api() const override; ::grpc::Status SetPower( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/movement_sensor/server.cpp b/src/viam/sdk/components/movement_sensor/server.cpp index 05da89ad8..a38de44e9 100644 --- a/src/viam/sdk/components/movement_sensor/server.cpp +++ b/src/viam/sdk/components/movement_sensor/server.cpp @@ -140,9 +140,5 @@ ::grpc::Status MovementSensorServer::GetGeometries( }); } -API MovementSensorServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/movement_sensor/server.hpp b/src/viam/sdk/components/movement_sensor/server.hpp index a4a46658f..c0ad7e690 100644 --- a/src/viam/sdk/components/movement_sensor/server.hpp +++ b/src/viam/sdk/components/movement_sensor/server.hpp @@ -20,7 +20,6 @@ class MovementSensorServer public viam::component::movementsensor::v1::MovementSensorService::Service { public: explicit MovementSensorServer(std::shared_ptr manager); - API api() const override; ::grpc::Status GetLinearVelocity( ::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/power_sensor/server.cpp b/src/viam/sdk/components/power_sensor/server.cpp index dc393702c..fb16abd8c 100644 --- a/src/viam/sdk/components/power_sensor/server.cpp +++ b/src/viam/sdk/components/power_sensor/server.cpp @@ -68,9 +68,5 @@ ::grpc::Status PowerSensorServer::DoCommand( }); } -API PowerSensorServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/power_sensor/server.hpp b/src/viam/sdk/components/power_sensor/server.hpp index d49bfcf2e..ac6035612 100644 --- a/src/viam/sdk/components/power_sensor/server.hpp +++ b/src/viam/sdk/components/power_sensor/server.hpp @@ -20,7 +20,6 @@ namespace sdk { class PowerSensorServer : public ResourceServer, public PowerSensorService::Service { public: explicit PowerSensorServer(std::shared_ptr manager); - API api() const override; ::grpc::Status GetVoltage(::grpc::ServerContext* context, const GetVoltageRequest* request, diff --git a/src/viam/sdk/components/sensor/server.cpp b/src/viam/sdk/components/sensor/server.cpp index ba6231968..eab3859aa 100644 --- a/src/viam/sdk/components/sensor/server.cpp +++ b/src/viam/sdk/components/sensor/server.cpp @@ -48,9 +48,5 @@ ::grpc::Status SensorServer::GetGeometries(::grpc::ServerContext* context, }); } -API SensorServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/sensor/server.hpp b/src/viam/sdk/components/sensor/server.hpp index 0caaae7ad..41baafbc9 100644 --- a/src/viam/sdk/components/sensor/server.hpp +++ b/src/viam/sdk/components/sensor/server.hpp @@ -21,7 +21,6 @@ class SensorServer : public ResourceServer, public viam::component::sensor::v1::SensorService::Service { public: explicit SensorServer(std::shared_ptr manager); - API api() const override; ::grpc::Status GetReadings(::grpc::ServerContext* context, const GetReadingsRequest* request, diff --git a/src/viam/sdk/components/servo/server.cpp b/src/viam/sdk/components/servo/server.cpp index b4ad859a1..8ac254204 100644 --- a/src/viam/sdk/components/servo/server.cpp +++ b/src/viam/sdk/components/servo/server.cpp @@ -71,9 +71,5 @@ ::grpc::Status ServoServer::DoCommand(grpc::ServerContext* context, }); } -API ServoServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/components/servo/server.hpp b/src/viam/sdk/components/servo/server.hpp index ad606d071..e8ed6ff34 100644 --- a/src/viam/sdk/components/servo/server.hpp +++ b/src/viam/sdk/components/servo/server.hpp @@ -19,7 +19,6 @@ class ServoServer : public ResourceServer, public viam::component::servo::v1::ServoService::Service { public: explicit ServoServer(std::shared_ptr manager); - API api() const override; ::grpc::Status Move(::grpc::ServerContext* context, const ::viam::component::servo::v1::MoveRequest* request, diff --git a/src/viam/sdk/resource/resource_server_base.cpp b/src/viam/sdk/resource/resource_server_base.cpp index d80b225ce..9f966680a 100644 --- a/src/viam/sdk/resource/resource_server_base.cpp +++ b/src/viam/sdk/resource/resource_server_base.cpp @@ -6,9 +6,5 @@ const std::shared_ptr& ResourceServer::resource_manager() const return manager_; }; -std::shared_ptr& ResourceServer::resource_manager() { - return manager_; -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/resource/resource_server_base.hpp b/src/viam/sdk/resource/resource_server_base.hpp index 581b9a9b8..a7a954090 100644 --- a/src/viam/sdk/resource/resource_server_base.hpp +++ b/src/viam/sdk/resource/resource_server_base.hpp @@ -7,9 +7,7 @@ namespace sdk { class ResourceServer { public: - virtual API api() const = 0; const std::shared_ptr& resource_manager() const; - std::shared_ptr& resource_manager(); protected: ResourceServer(std::shared_ptr manager) : manager_(manager){}; diff --git a/src/viam/sdk/robot/service.cpp b/src/viam/sdk/robot/service.cpp index 94d3328fe..5fabc88bd 100644 --- a/src/viam/sdk/robot/service.cpp +++ b/src/viam/sdk/robot/service.cpp @@ -213,9 +213,5 @@ std::shared_ptr RobotService_::resource_by_name(Name name) { return r; } -API RobotService_::api() const { - return {"RDK", "robot", "robot"}; -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/robot/service.hpp b/src/viam/sdk/robot/service.hpp index 902c54271..2864b2caa 100644 --- a/src/viam/sdk/robot/service.hpp +++ b/src/viam/sdk/robot/service.hpp @@ -49,7 +49,6 @@ class RobotService_ : public ResourceServer, public viam::robot::v1::RobotServic ::viam::robot::v1::StopAllResponse* response) override; private: - API api() const override; std::mutex lock_; std::vector generate_metadata(); std::vector generate_status(RepeatedPtrField resource_names); diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index b050e41e0..bdb190b51 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -15,7 +15,7 @@ Server::Server() : builder_(std::make_unique()) { for (const auto& rr : Registry::registered_resources()) { auto new_manager = std::make_shared(); auto server = rr.second->create_resource_server(new_manager, *this); - managed_servers_.emplace(server->api(), std::move(server)); + managed_servers_.emplace(rr.first, std::move(server)); } } diff --git a/src/viam/sdk/services/generic/server.cpp b/src/viam/sdk/services/generic/server.cpp index ea983745b..5403d41b3 100644 --- a/src/viam/sdk/services/generic/server.cpp +++ b/src/viam/sdk/services/generic/server.cpp @@ -21,9 +21,5 @@ ::grpc::Status GenericServiceServer::DoCommand( }); } -API GenericServiceServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/services/generic/server.hpp b/src/viam/sdk/services/generic/server.hpp index b98d48a0a..f72ab2b94 100644 --- a/src/viam/sdk/services/generic/server.hpp +++ b/src/viam/sdk/services/generic/server.hpp @@ -19,7 +19,6 @@ class GenericServiceServer : public ResourceServer, public viam::service::generic::v1::GenericService::Service { public: explicit GenericServiceServer(std::shared_ptr manager); - API api() const override; ::grpc::Status DoCommand(::grpc::ServerContext* context, const ::viam::common::v1::DoCommandRequest* request, diff --git a/src/viam/sdk/services/mlmodel/server.cpp b/src/viam/sdk/services/mlmodel/server.cpp index 3a269f0bf..50dbfb37b 100644 --- a/src/viam/sdk/services/mlmodel/server.cpp +++ b/src/viam/sdk/services/mlmodel/server.cpp @@ -158,9 +158,5 @@ ::grpc::Status MLModelServiceServer::Metadata( }); } -API MLModelServiceServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/services/mlmodel/server.hpp b/src/viam/sdk/services/mlmodel/server.hpp index 7679ced29..e4ac1f8b8 100644 --- a/src/viam/sdk/services/mlmodel/server.hpp +++ b/src/viam/sdk/services/mlmodel/server.hpp @@ -30,7 +30,6 @@ class MLModelServiceServer : public ResourceServer, public ::viam::service::mlmodel::v1::MLModelService::Service { public: explicit MLModelServiceServer(std::shared_ptr manager); - API api() const override; ::grpc::Status Infer(::grpc::ServerContext* context, const ::viam::service::mlmodel::v1::InferRequest* request, diff --git a/src/viam/sdk/services/motion/server.cpp b/src/viam/sdk/services/motion/server.cpp index 20580fdc3..94edd590b 100644 --- a/src/viam/sdk/services/motion/server.cpp +++ b/src/viam/sdk/services/motion/server.cpp @@ -212,9 +212,5 @@ ::grpc::Status MotionServer::DoCommand(::grpc::ServerContext* context, }); }; -API MotionServer::api() const { - return API::get(); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/services/motion/server.hpp b/src/viam/sdk/services/motion/server.hpp index 1db6d8b4d..cc145f4fe 100644 --- a/src/viam/sdk/services/motion/server.hpp +++ b/src/viam/sdk/services/motion/server.hpp @@ -21,7 +21,6 @@ class MotionServer : public ResourceServer, public viam::service::motion::v1::MotionService::Service { public: explicit MotionServer(std::shared_ptr manager); - API api() const override; ::grpc::Status Move(::grpc::ServerContext* context, const ::viam::service::motion::v1::MoveRequest* request, From f938ec58976a3bec08f48d8564bcca512bc5b41e Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Thu, 8 Feb 2024 11:37:48 -0500 Subject: [PATCH 15/20] PR comments --- src/viam/examples/modules/complex/client.cpp | 7 +- .../examples/modules/complex/gizmo/api.hpp | 3 + src/viam/examples/modules/complex/main.cpp | 13 +- .../examples/modules/complex/proto/buf.lock | 4 +- .../modules/complex/summation/api.hpp | 3 + .../modules/complex/test_complex_module.cpp | 5 +- src/viam/sdk/common/utils.cpp | 2 +- src/viam/sdk/components/base/client.hpp | 1 + src/viam/sdk/components/base/server.hpp | 3 + src/viam/sdk/components/board/client.hpp | 1 + src/viam/sdk/components/board/server.hpp | 3 + src/viam/sdk/components/camera/client.hpp | 1 + src/viam/sdk/components/camera/server.hpp | 3 + src/viam/sdk/components/component.cpp | 2 +- src/viam/sdk/components/component.hpp | 2 +- src/viam/sdk/components/encoder/client.hpp | 1 + src/viam/sdk/components/encoder/server.hpp | 3 + src/viam/sdk/components/generic/client.hpp | 1 + src/viam/sdk/components/generic/server.hpp | 3 + src/viam/sdk/components/motor/client.hpp | 1 + src/viam/sdk/components/motor/server.hpp | 3 + .../sdk/components/movement_sensor/client.hpp | 1 + .../sdk/components/movement_sensor/server.hpp | 3 + .../sdk/components/power_sensor/client.hpp | 1 + .../sdk/components/power_sensor/server.hpp | 3 + src/viam/sdk/components/sensor/client.hpp | 1 + src/viam/sdk/components/sensor/server.hpp | 3 + src/viam/sdk/components/servo/client.hpp | 1 + src/viam/sdk/components/servo/server.hpp | 3 + src/viam/sdk/module/service.cpp | 11 +- src/viam/sdk/registry/registry.cpp | 117 ++++++++------- src/viam/sdk/registry/registry.hpp | 137 +++++++++++------- src/viam/sdk/resource/resource.cpp | 2 +- src/viam/sdk/resource/resource.hpp | 2 +- src/viam/sdk/robot/client.cpp | 4 +- src/viam/sdk/robot/service.cpp | 2 +- src/viam/sdk/rpc/server.cpp | 2 +- src/viam/sdk/services/generic/client.hpp | 1 + src/viam/sdk/services/generic/server.hpp | 3 + src/viam/sdk/services/mlmodel/client.hpp | 1 + src/viam/sdk/services/mlmodel/server.hpp | 3 + src/viam/sdk/services/motion/client.hpp | 1 + src/viam/sdk/services/motion/server.hpp | 3 + src/viam/sdk/services/service.cpp | 2 +- src/viam/sdk/services/service.hpp | 2 +- src/viam/sdk/tests/mocks/mock_robot.cpp | 1 - 46 files changed, 232 insertions(+), 143 deletions(-) diff --git a/src/viam/examples/modules/complex/client.cpp b/src/viam/examples/modules/complex/client.cpp index 55d8fd58a..ba9d53a64 100644 --- a/src/viam/examples/modules/complex/client.cpp +++ b/src/viam/examples/modules/complex/client.cpp @@ -37,11 +37,10 @@ int main() { std::string address(uri); Options options(1, opts); - // Register custom gizmo and summation API so robot client can access resources + // Register custom gizmo and summation clients so robot client can access resources // of that type from the server. - Registry::register_resource(API::get()); - Registry::register_resource( - API::get()); + Registry::register_resource_client(); + Registry::register_resource_client(); // Connect to robot. std::shared_ptr robot = RobotClient::at_address(address, options); diff --git a/src/viam/examples/modules/complex/gizmo/api.hpp b/src/viam/examples/modules/complex/gizmo/api.hpp index a9b5fcb68..fa5cc44ba 100644 --- a/src/viam/examples/modules/complex/gizmo/api.hpp +++ b/src/viam/examples/modules/complex/gizmo/api.hpp @@ -40,6 +40,7 @@ struct API::traits { // `GizmoClient` is the gRPC client implementation of a `Gizmo` component. class GizmoClient : public Gizmo { public: + using interface_type = Gizmo; GizmoClient(std::string name, std::shared_ptr channel); bool do_one(std::string arg1) override; @@ -57,6 +58,8 @@ class GizmoClient : public Gizmo { // `GizmoServer` is the gRPC server implementation of a `Gizmo` component. class GizmoServer : public ResourceServer, public GizmoService::Service { public: + using interface_type = Gizmo; + using service_type = GizmoService; explicit GizmoServer(std::shared_ptr manager); grpc::Status DoOne(grpc::ServerContext* context, diff --git a/src/viam/examples/modules/complex/main.cpp b/src/viam/examples/modules/complex/main.cpp index 4d6558ca3..c8f447224 100644 --- a/src/viam/examples/modules/complex/main.cpp +++ b/src/viam/examples/modules/complex/main.cpp @@ -28,24 +28,21 @@ using namespace viam::sdk; int main(int argc, char** argv) { - API base_api = API::get(); - API gizmo_api = API::get(); - API summation_api = API::get(); Model mybase_model("viam", "base", "mybase"); // Make sure to explicity register resources with custom APIs. - Registry::register_resource(gizmo_api); - Registry::register_resource(summation_api); + Registry::register_resource_server(); + Registry::register_resource_server(); std::shared_ptr mybase_mr = std::make_shared( - base_api, + API::get(), mybase_model, [](Dependencies deps, ResourceConfig cfg) { return std::make_unique(deps, cfg); }, MyBase::validate); Model mygizmo_model("viam", "gizmo", "mygizmo"); std::shared_ptr mygizmo_mr = std::make_shared( - gizmo_api, + API::get(), mygizmo_model, [](Dependencies deps, ResourceConfig cfg) { return std::make_unique(deps, cfg); }, MyGizmo::validate); @@ -53,7 +50,7 @@ int main(int argc, char** argv) { Model mysummation_model("viam", "summation", "mysummation"); std::shared_ptr mysummation_mr = std::make_shared( - summation_api, mysummation_model, [](Dependencies deps, ResourceConfig cfg) { + API::get(), mysummation_model, [](Dependencies deps, ResourceConfig cfg) { return std::make_unique(deps, cfg); }); diff --git a/src/viam/examples/modules/complex/proto/buf.lock b/src/viam/examples/modules/complex/proto/buf.lock index 013f46cac..b46be1102 100644 --- a/src/viam/examples/modules/complex/proto/buf.lock +++ b/src/viam/examples/modules/complex/proto/buf.lock @@ -4,5 +4,5 @@ deps: - remote: buf.build owner: googleapis repository: googleapis - commit: e874a0be2bf140a5a4c7d4122c635823 - digest: shake256:4fe3036b4d706f6ee2b13c730bd04777f021dfd02ed27e6e40480acfe664a7548238312ee0727fd77648a38d227e296d43f4a38a34cdd46068156211016d9657 + commit: 7e6f6e774e29406da95bd61cdcdbc8bc + digest: shake256:fe43dd2265ea0c07d76bd925eeba612667cf4c948d2ce53d6e367e1b4b3cb5fa69a51e6acb1a6a50d32f894f054a35e6c0406f6808a483f2752e10c866ffbf73 diff --git a/src/viam/examples/modules/complex/summation/api.hpp b/src/viam/examples/modules/complex/summation/api.hpp index 2f253525b..923b75a5a 100644 --- a/src/viam/examples/modules/complex/summation/api.hpp +++ b/src/viam/examples/modules/complex/summation/api.hpp @@ -40,6 +40,7 @@ struct API::traits { // service. class SummationClient : public Summation { public: + using interface_type = Summation; SummationClient(std::string name, std::shared_ptr channel); double sum(std::vector numbers) override; @@ -54,6 +55,8 @@ class SummationClient : public Summation { // service. class SummationServer : public ResourceServer, public SummationService::Service { public: + using interface_type = Summation; + using service_type = SummationService; explicit SummationServer(std::shared_ptr manager); grpc::Status Sum(grpc::ServerContext* context, diff --git a/src/viam/examples/modules/complex/test_complex_module.cpp b/src/viam/examples/modules/complex/test_complex_module.cpp index 0b90d49a2..840b42411 100644 --- a/src/viam/examples/modules/complex/test_complex_module.cpp +++ b/src/viam/examples/modules/complex/test_complex_module.cpp @@ -26,9 +26,8 @@ using namespace viam::sdktests; struct RegisterGizmoAndSummationFixture { RegisterGizmoAndSummationFixture() { - Registry::register_resource(API::get()); - Registry::register_resource( - API::get()); + Registry::register_resource(); + Registry::register_resource(); } // Test teardown is a noop; diff --git a/src/viam/sdk/common/utils.cpp b/src/viam/sdk/common/utils.cpp index 04e9260c4..a554eda65 100644 --- a/src/viam/sdk/common/utils.cpp +++ b/src/viam/sdk/common/utils.cpp @@ -29,7 +29,7 @@ std::vector resource_names_for_resource(const std::shared_ptr resource_names; for (const auto& kv : Registry::registered_models()) { - const std::shared_ptr reg = kv.second; + const std::shared_ptr reg = kv.second; if (reg->api().to_string() == resource->api().to_string()) { resource_type = reg->api().resource_type(); resource_subtype = reg->api().resource_subtype(); diff --git a/src/viam/sdk/components/base/client.hpp b/src/viam/sdk/components/base/client.hpp index 8aec85262..ffcf62ac3 100644 --- a/src/viam/sdk/components/base/client.hpp +++ b/src/viam/sdk/components/base/client.hpp @@ -22,6 +22,7 @@ namespace sdk { /// @ingroup Base class BaseClient : public Base { public: + using interface_type = Base; BaseClient(std::string name, std::shared_ptr channel); void move_straight(int64_t distance_mm, double mm_per_sec, const AttributeMap& extra) override; void spin(double angle_deg, double degs_per_sec, const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/base/server.hpp b/src/viam/sdk/components/base/server.hpp index 6755979ec..c1046c27a 100644 --- a/src/viam/sdk/components/base/server.hpp +++ b/src/viam/sdk/components/base/server.hpp @@ -7,6 +7,7 @@ #include #include +#include #include #include @@ -18,6 +19,8 @@ namespace sdk { /// @ingroup Base class BaseServer : public ResourceServer, public viam::component::base::v1::BaseService::Service { public: + using interface_type = Base; + using service_type = component::base::v1::BaseService; explicit BaseServer(std::shared_ptr manager); ::grpc::Status MoveStraight( diff --git a/src/viam/sdk/components/board/client.hpp b/src/viam/sdk/components/board/client.hpp index 4a7e0d058..b754beb2e 100644 --- a/src/viam/sdk/components/board/client.hpp +++ b/src/viam/sdk/components/board/client.hpp @@ -20,6 +20,7 @@ namespace sdk { /// @ingroup Board class BoardClient : public Board { public: + using interface_type = Board; BoardClient(std::string name, std::shared_ptr channel); AttributeMap do_command(const AttributeMap& command) override; status get_status(const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/board/server.hpp b/src/viam/sdk/components/board/server.hpp index 8bf5a6c39..4ad282243 100644 --- a/src/viam/sdk/components/board/server.hpp +++ b/src/viam/sdk/components/board/server.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -18,6 +19,8 @@ namespace sdk { class BoardServer : public ResourceServer, public viam::component::board::v1::BoardService::Service { public: + using service_type = component::board::v1::BoardService; + using interface_type = Board; explicit BoardServer(std::shared_ptr manager); ::grpc::Status Status(::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/camera/client.hpp b/src/viam/sdk/components/camera/client.hpp index 1e6772dfa..d05c4866e 100644 --- a/src/viam/sdk/components/camera/client.hpp +++ b/src/viam/sdk/components/camera/client.hpp @@ -21,6 +21,7 @@ namespace sdk { /// @ingroup Camera class CameraClient : public Camera { public: + using interface_type = Camera; CameraClient(std::string name, std::shared_ptr channel); AttributeMap do_command(AttributeMap command) override; raw_image get_image(std::string mime_type, const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/camera/server.hpp b/src/viam/sdk/components/camera/server.hpp index a7ffdd3b2..c042b75c5 100644 --- a/src/viam/sdk/components/camera/server.hpp +++ b/src/viam/sdk/components/camera/server.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -18,6 +19,8 @@ namespace sdk { class CameraServer : public ResourceServer, public viam::component::camera::v1::CameraService::Service { public: + using interface_type = Camera; + using service_type = component::camera::v1::CameraService; explicit CameraServer(std::shared_ptr manager); ::grpc::Status DoCommand(::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/component.cpp b/src/viam/sdk/components/component.cpp index 23d17d5a6..c44bd91e9 100644 --- a/src/viam/sdk/components/component.cpp +++ b/src/viam/sdk/components/component.cpp @@ -18,7 +18,7 @@ Component::Component() : Resource("component"){}; Component::Component(std::string name) : Resource(std::move(name)){}; -ResourceName Component::get_resource_name(std::string name) { +ResourceName Component::get_resource_name(std::string name) const { auto r = this->Resource::get_resource_name(name); *r.mutable_type() = kComponent; return r; diff --git a/src/viam/sdk/components/component.hpp b/src/viam/sdk/components/component.hpp index 26b982f93..83830d2e4 100644 --- a/src/viam/sdk/components/component.hpp +++ b/src/viam/sdk/components/component.hpp @@ -14,7 +14,7 @@ namespace sdk { class Component : public Resource { public: Component(); - viam::common::v1::ResourceName get_resource_name(std::string name) override; + viam::common::v1::ResourceName get_resource_name(std::string name) const override; protected: explicit Component(std::string name); diff --git a/src/viam/sdk/components/encoder/client.hpp b/src/viam/sdk/components/encoder/client.hpp index 3b7611394..786cf1b33 100644 --- a/src/viam/sdk/components/encoder/client.hpp +++ b/src/viam/sdk/components/encoder/client.hpp @@ -20,6 +20,7 @@ namespace sdk { /// @ingroup Encoder class EncoderClient : public Encoder { public: + using interface_type = Encoder; EncoderClient(std::string name, std::shared_ptr channel); position get_position(const AttributeMap& extra, position_type position_type) override; void reset_position(const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/encoder/server.hpp b/src/viam/sdk/components/encoder/server.hpp index c67c59e43..131a871c7 100644 --- a/src/viam/sdk/components/encoder/server.hpp +++ b/src/viam/sdk/components/encoder/server.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -18,6 +19,8 @@ namespace sdk { class EncoderServer : public ResourceServer, public viam::component::encoder::v1::EncoderService::Service { public: + using interface_type = Encoder; + using service_type = component::encoder::v1::EncoderService; explicit EncoderServer(std::shared_ptr manager); ::grpc::Status GetPosition( diff --git a/src/viam/sdk/components/generic/client.hpp b/src/viam/sdk/components/generic/client.hpp index 97a4b0168..8e24a3b03 100644 --- a/src/viam/sdk/components/generic/client.hpp +++ b/src/viam/sdk/components/generic/client.hpp @@ -18,6 +18,7 @@ namespace sdk { /// @ingroup GenericComponent class GenericComponentClient : public GenericComponent { public: + using interface_type = GenericComponent; GenericComponentClient(std::string name, std::shared_ptr channel); AttributeMap do_command(AttributeMap command) override; std::vector get_geometries(const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/generic/server.hpp b/src/viam/sdk/components/generic/server.hpp index d1061b9c0..bd59e7715 100644 --- a/src/viam/sdk/components/generic/server.hpp +++ b/src/viam/sdk/components/generic/server.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -18,6 +19,8 @@ namespace sdk { class GenericComponentServer : public ResourceServer, public viam::component::generic::v1::GenericService::Service { public: + using interface_type = GenericComponent; + using service_type = component::generic::v1::GenericService; explicit GenericComponentServer(std::shared_ptr manager); ::grpc::Status DoCommand(::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/motor/client.hpp b/src/viam/sdk/components/motor/client.hpp index 147535e80..fc9769b4d 100644 --- a/src/viam/sdk/components/motor/client.hpp +++ b/src/viam/sdk/components/motor/client.hpp @@ -19,6 +19,7 @@ namespace sdk { /// @ingroup Motor class MotorClient : public Motor { public: + using interface_type = Motor; MotorClient(std::string name, std::shared_ptr channel); void set_power(double power_pct, const AttributeMap& extra) override; void go_for(double rpm, double revolutions, const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/motor/server.hpp b/src/viam/sdk/components/motor/server.hpp index 7e2a22c94..eb4c4db05 100644 --- a/src/viam/sdk/components/motor/server.hpp +++ b/src/viam/sdk/components/motor/server.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -18,6 +19,8 @@ namespace sdk { class MotorServer : public ResourceServer, public viam::component::motor::v1::MotorService::Service { public: + using interface_type = Motor; + using service_type = component::motor::v1::MotorService; explicit MotorServer(std::shared_ptr manager); ::grpc::Status SetPower( diff --git a/src/viam/sdk/components/movement_sensor/client.hpp b/src/viam/sdk/components/movement_sensor/client.hpp index e17f33f47..5f23e2117 100644 --- a/src/viam/sdk/components/movement_sensor/client.hpp +++ b/src/viam/sdk/components/movement_sensor/client.hpp @@ -21,6 +21,7 @@ namespace sdk { /// @ingroup MovementSensor class MovementSensorClient : public MovementSensor { public: + using interface_type = MovementSensor; MovementSensorClient(std::string name, std::shared_ptr channel); Vector3 get_linear_velocity(const AttributeMap& extra) override; Vector3 get_angular_velocity(const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/movement_sensor/server.hpp b/src/viam/sdk/components/movement_sensor/server.hpp index c0ad7e690..023954c3e 100644 --- a/src/viam/sdk/components/movement_sensor/server.hpp +++ b/src/viam/sdk/components/movement_sensor/server.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -19,6 +20,8 @@ class MovementSensorServer : public ResourceServer, public viam::component::movementsensor::v1::MovementSensorService::Service { public: + using interface_type = MovementSensor; + using service_type = component::movementsensor::v1::MovementSensorService; explicit MovementSensorServer(std::shared_ptr manager); ::grpc::Status GetLinearVelocity( diff --git a/src/viam/sdk/components/power_sensor/client.hpp b/src/viam/sdk/components/power_sensor/client.hpp index c710936cd..3d638f1cc 100644 --- a/src/viam/sdk/components/power_sensor/client.hpp +++ b/src/viam/sdk/components/power_sensor/client.hpp @@ -20,6 +20,7 @@ namespace sdk { /// @ingroup PowerSensor class PowerSensorClient : public PowerSensor { public: + using interface_type = PowerSensor; PowerSensorClient(std::string name, std::shared_ptr channel); voltage get_voltage(const AttributeMap& extra) override; current get_current(const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/power_sensor/server.hpp b/src/viam/sdk/components/power_sensor/server.hpp index ac6035612..0ae682916 100644 --- a/src/viam/sdk/components/power_sensor/server.hpp +++ b/src/viam/sdk/components/power_sensor/server.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -19,6 +20,8 @@ namespace sdk { /// @ingroup PowerSensor class PowerSensorServer : public ResourceServer, public PowerSensorService::Service { public: + using interface_type = PowerSensor; + using service_type = PowerSensorService; explicit PowerSensorServer(std::shared_ptr manager); ::grpc::Status GetVoltage(::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/sensor/client.hpp b/src/viam/sdk/components/sensor/client.hpp index 0d2f5eb28..aca78a2d0 100644 --- a/src/viam/sdk/components/sensor/client.hpp +++ b/src/viam/sdk/components/sensor/client.hpp @@ -19,6 +19,7 @@ namespace sdk { /// @ingroup Sensor class SensorClient : public Sensor { public: + using interface_type = Sensor; SensorClient(std::string name, std::shared_ptr channel); AttributeMap get_readings(const AttributeMap& extra) override; AttributeMap do_command(const AttributeMap& command) override; diff --git a/src/viam/sdk/components/sensor/server.hpp b/src/viam/sdk/components/sensor/server.hpp index 41baafbc9..c581e4e4f 100644 --- a/src/viam/sdk/components/sensor/server.hpp +++ b/src/viam/sdk/components/sensor/server.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -20,6 +21,8 @@ namespace sdk { class SensorServer : public ResourceServer, public viam::component::sensor::v1::SensorService::Service { public: + using interface_type = Sensor; + using service_type = component::sensor::v1::SensorService; explicit SensorServer(std::shared_ptr manager); ::grpc::Status GetReadings(::grpc::ServerContext* context, diff --git a/src/viam/sdk/components/servo/client.hpp b/src/viam/sdk/components/servo/client.hpp index fa5f2c10b..df7d1f238 100644 --- a/src/viam/sdk/components/servo/client.hpp +++ b/src/viam/sdk/components/servo/client.hpp @@ -20,6 +20,7 @@ namespace sdk { /// @ingroup Servo class ServoClient : public Servo { public: + using interface_type = Servo; ServoClient(std::string name, std::shared_ptr channel); void move(uint32_t angle_deg, const AttributeMap& extra) override; position get_position(const AttributeMap& extra) override; diff --git a/src/viam/sdk/components/servo/server.hpp b/src/viam/sdk/components/servo/server.hpp index e8ed6ff34..4556ce311 100644 --- a/src/viam/sdk/components/servo/server.hpp +++ b/src/viam/sdk/components/servo/server.hpp @@ -6,6 +6,7 @@ #include #include +#include #include #include @@ -18,6 +19,8 @@ namespace sdk { class ServoServer : public ResourceServer, public viam::component::servo::v1::ServoService::Service { public: + using interface_type = Servo; + using service_type = component::servo::v1::ServoService; explicit ServoServer(std::shared_ptr manager); ::grpc::Status Move(::grpc::ServerContext* context, diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 6a8fa6b6f..90217bbb9 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -79,7 +79,8 @@ ::grpc::Status ModuleService::AddResource(::grpc::ServerContext* context, std::shared_ptr res; const Dependencies deps = get_dependencies_(request->dependencies(), cfg.name()); - const std::shared_ptr reg = Registry::lookup_model(cfg.api(), cfg.model()); + const std::shared_ptr reg = + Registry::lookup_model(cfg.api(), cfg.model()); if (reg) { try { res = reg->construct_resource(deps, cfg); @@ -132,7 +133,7 @@ ::grpc::Status ModuleService::ReconfigureResource( BOOST_LOG_TRIVIAL(error) << "unable to stop resource: " << err.what(); } - const std::shared_ptr reg = Registry::lookup_model(cfg.name()); + const std::shared_ptr reg = Registry::lookup_model(cfg.name()); if (reg) { try { const std::shared_ptr res = reg->construct_resource(deps, cfg); @@ -152,7 +153,8 @@ ::grpc::Status ModuleService::ValidateConfig( const viam::app::v1::ComponentConfig& proto = request->config(); ResourceConfig cfg = ResourceConfig::from_proto(proto); - const std::shared_ptr reg = Registry::lookup_model(cfg.api(), cfg.model()); + const std::shared_ptr reg = + Registry::lookup_model(cfg.api(), cfg.model()); if (!reg) { return grpc::Status(grpc::UNKNOWN, "unable to validate resource " + cfg.resource_name().name() + @@ -265,7 +267,8 @@ ModuleService::~ModuleService() { void ModuleService::add_model_from_registry_inlock_(API api, Model model, const std::lock_guard& lock) { - const std::shared_ptr creator = Registry::lookup_resource(api); + const std::shared_ptr creator = + Registry::lookup_resource_server(api); std::string name; const google::protobuf::ServiceDescriptor* sd = nullptr; if (creator && creator->service_descriptor()) { diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 3a6d6c976..b5e4d6531 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -74,7 +74,8 @@ namespace sdk { using viam::robot::v1::Status; -ResourceRegistration::~ResourceRegistration() = default; +ResourceServerRegistration::~ResourceServerRegistration() = default; +ResourceClientRegistration::~ResourceClientRegistration() = default; const API& ModelRegistration::api() const { return api_; @@ -83,7 +84,7 @@ const Model& ModelRegistration::model() const { return model_; }; -void Registry::register_model(std::shared_ptr resource) { +void Registry::register_model(std::shared_ptr resource) { const std::string reg_key = resource->api().to_string() + "/" + resource->model().to_string(); if (resources_.find(reg_key) != resources_.end()) { const std::string err = "Cannot add resource with name " + reg_key + "as it already exists"; @@ -93,16 +94,26 @@ void Registry::register_model(std::shared_ptr resource) { resources_.emplace(reg_key, resource); } -void Registry::register_resource(API api, - std::shared_ptr resource_registration) { - if (apis_.find(api) != apis_.end()) { +void Registry::register_resource_server_( + API api, std::shared_ptr resource_registration) { + if (server_apis_.find(api) != server_apis_.end()) { throw std::runtime_error("Cannot add api " + api.to_string() + " as it already exists"); } - apis_.emplace(std::move(api), std::move(resource_registration)); + server_apis_.emplace(std::move(api), std::move(resource_registration)); } -std::shared_ptr Registry::lookup_model(std::string name) { +void Registry::register_resource_client_( + API api, std::shared_ptr resource_registration) { + if (client_apis_.find(api) != client_apis_.end()) { + throw std::runtime_error("Cannot add api " + api.to_string() + " as it already exists"); + } + + client_apis_.emplace(std::move(api), std::move(resource_registration)); +} + +std::shared_ptr Registry::lookup_model_inlock_( + std::string name, const std::lock_guard&) { if (resources_.find(name) == resources_.end()) { return nullptr; } @@ -110,17 +121,33 @@ std::shared_ptr Registry::lookup_model(std::string name) { return resources_.at(name); } -std::shared_ptr Registry::lookup_model(API api, Model model) { +std::shared_ptr Registry::lookup_model(std::string name) { + const std::lock_guard lock(lock_); + return lookup_model_inlock_(name, lock); +} + +std::shared_ptr Registry::lookup_model(API api, Model model) { + const std::lock_guard lock(lock_); const std::string name = api.to_string() + "/" + model.to_string(); - return lookup_model(name); + return lookup_model_inlock_(name, lock); +} + +std::shared_ptr Registry::lookup_resource_server(API api) { + const std::lock_guard lock(lock_); + if (server_apis_.find(api) == server_apis_.end()) { + return nullptr; + } + + return server_apis_.at(api); } -std::shared_ptr Registry::lookup_resource(API api) { - if (apis_.find(api) == apis_.end()) { +std::shared_ptr Registry::lookup_resource_client(API api) { + const std::lock_guard lock(lock_); + if (client_apis_.find(api) == client_apis_.end()) { return nullptr; } - return apis_.at(api); + return client_apis_.at(api); } const google::protobuf::ServiceDescriptor* Registry::get_service_descriptor_( @@ -133,67 +160,45 @@ const google::protobuf::ServiceDescriptor* Registry::get_service_descriptor_( return sd; } -const std::unordered_map>& -Registry::registered_resources() { - return apis_; +const std::unordered_map>& +Registry::registered_resource_servers() { + return server_apis_; } -const std::unordered_map>& +const std::unordered_map>& Registry::registered_models() { return resources_; } // NOLINTNEXTLINE(readability-convert-member-functions-to-static) -Status ModelRegistration::create_status(std::shared_ptr resource) { +Status ModelRegistration::create_status(std::shared_ptr resource) const { Status status; *status.mutable_name() = resource->get_resource_name(resource->name()); *status.mutable_status() = google::protobuf::Struct(); return status; } -const google::protobuf::ServiceDescriptor* ResourceRegistration::service_descriptor() { +const google::protobuf::ServiceDescriptor* ResourceServerRegistration::service_descriptor() const { return service_descriptor_; } void register_resources() { // Register all components - Registry::register_resource( - API::get()); - Registry::register_resource( - API::get()); - Registry::register_resource( - API::get()); - Registry::register_resource(API::get()); - Registry::register_resource( - API::get()); - Registry::register_resource( - API::get()); - Registry::register_resource( - API::get()); - Registry::register_resource( - API::get()); - Registry::register_resource( - API::get()); - Registry::register_resource( - API::get()); + Registry::register_resource(); + Registry::register_resource(); + Registry::register_resource(); + Registry::register_resource(); + Registry::register_resource(); + Registry::register_resource(); + Registry::register_resource(); + Registry::register_resource(); + Registry::register_resource(); + Registry::register_resource(); // Register all services - Registry::register_resource(API::get()); - Registry::register_resource(API::get()); - Registry::register_resource( - API::get()); + Registry::register_resource(); + Registry::register_resource(); + Registry::register_resource(); } void Registry::initialize() { @@ -207,8 +212,10 @@ void Registry::initialize() { grpc::reflection::InitProtoReflectionServerBuilderPlugin(); } -std::unordered_map> Registry::resources_; -std::unordered_map> Registry::apis_; +std::unordered_map> Registry::resources_; +std::unordered_map> Registry::client_apis_; +std::unordered_map> Registry::server_apis_; +std::mutex Registry::lock_; bool Registry::initialized_{false}; } // namespace sdk diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index b13572d8a..5685e9c67 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -11,8 +11,6 @@ #include #include -#include - #include #include #include @@ -26,41 +24,41 @@ namespace sdk { // TODO(RSDK-3030): instead of std::functions, consider making these functions // virtual // TODO(RSDK-3030): one class per header -/// @class ResourceRegistration registry.hpp "registry/registry.hpp" -/// @brief Defines registered `Resource` creation functionality. -class ResourceRegistration { +class ResourceServerRegistration { public: - virtual ~ResourceRegistration(); - - // TODO(RSDK-6484): the semantics of `create_reconfigurable` are opaque (and to-date - // are in fact non-existent). We should get rid of this and just add a `Reconfigurable` - // base class (similar to `Stoppable`), and things that want to reconfigure can inherit - // from it. - /// @brief Add `Reconfigure` functionality to a resource. - std::function(std::shared_ptr, Name)> create_reconfigurable; + virtual ~ResourceServerRegistration(); /// @brief Create a resource's gRPC server. /// @param manager The server's `ResourceManager`. /// @param server The Server with which to register the relevant gRPC service. /// @return a `shared_ptr` to the gRPC server. virtual std::shared_ptr create_resource_server( - std::shared_ptr manager, Server& server) = 0; + std::shared_ptr manager, Server& server) const = 0; - /// @brief Returns a reference to the `ResourceRegistration`'s service descriptor. - const google::protobuf::ServiceDescriptor* service_descriptor(); + /// @brief Returns a reference to the `ResourceServerRegistration`'s service descriptor. + const google::protobuf::ServiceDescriptor* service_descriptor() const; + + ResourceServerRegistration(const google::protobuf::ServiceDescriptor* service_descriptor) + : service_descriptor_(service_descriptor){}; + + private: + const google::protobuf::ServiceDescriptor* service_descriptor_; +}; + +/// @class ResourceClientRegistration registry.hpp "registry/registry.hpp" +/// @brief Defines registered `Resource` client creation functionality. +class ResourceClientRegistration { + public: + virtual ~ResourceClientRegistration(); /// @brief Create gRPC client to a resource. /// @param name The name of the resource. /// @param channel A channel connected to the client. /// @return A `shared_ptr` to the resource client. - virtual std::shared_ptr create_rpc_client(std::string name, - std::shared_ptr channel) = 0; - - ResourceRegistration(const google::protobuf::ServiceDescriptor* service_descriptor) - : service_descriptor_(service_descriptor){}; + virtual std::shared_ptr create_rpc_client( + std::string name, std::shared_ptr channel) const = 0; - private: - const google::protobuf::ServiceDescriptor* service_descriptor_; + ResourceClientRegistration() = default; }; /// @class ModelRegistration @@ -98,7 +96,7 @@ class ModelRegistration { std::function(ResourceConfig)> validate; /// @brief Creates a `Status` object for a given resource. - viam::robot::v1::Status create_status(std::shared_ptr resource); + viam::robot::v1::Status create_status(std::shared_ptr resource) const; private: // default_validator is the default validator for all models if no validator @@ -117,75 +115,104 @@ class Registry { /// @brief Registers a resource with the Registry. /// @param resource An object containing resource registration information. /// @throws `std::runtime_error` if the resource has already been registered. - static void register_model(std::shared_ptr resource); + static void register_model(std::shared_ptr resource); /// @brief Lookup a given registered resource. /// @param name The name of the resource to lookup. /// @return a `shared_ptr` to the resource's registration data. - static std::shared_ptr lookup_model(std::string name); + static std::shared_ptr lookup_model(std::string name); /// @brief Lookup a given registered resource. /// @param api The api of the resource to lookup. /// @param model The model of the resource to lookup. /// @return a `shared_ptr` to the resource's registration data. - static std::shared_ptr lookup_model(API api, Model model); + static std::shared_ptr lookup_model(API api, Model model); - /// @brief Register an api. - /// @param api The api to be registered. - template - static void register_resource(API api) { - class ResourceRegistration2 final : public ResourceRegistration { + /// @brief Register a resource client constructor + template + static void register_resource_client() { + class ResourceClientRegistration2 final : public ResourceClientRegistration { public: - using ResourceRegistration::ResourceRegistration; + using ResourceClientRegistration::ResourceClientRegistration; + + std::shared_ptr create_rpc_client( + std::string name, std::shared_ptr chan) const override { + return std::make_shared(std::move(name), std::move(chan)); + } + }; + + Registry::register_resource_client_(API::get(), + std::make_shared()); + } + + /// @brief Register a resource server constructor. + template + static void register_resource_server() { + class ResourceServerRegistration2 final : public ResourceServerRegistration { + public: + using ResourceServerRegistration::ResourceServerRegistration; std::shared_ptr create_resource_server( - std::shared_ptr manager, Server& server) override { + std::shared_ptr manager, Server& server) const override { auto rs = std::make_shared(manager); server.register_service(rs.get()); return rs; } - - std::shared_ptr create_rpc_client( - std::string name, std::shared_ptr chan) override { - return std::make_shared(std::move(name), std::move(chan)); - } }; const google::protobuf::ServiceDescriptor* sd = - get_service_descriptor_(ProtoServiceT::service_full_name()); - Registry::register_resource(api, std::make_shared(sd)); + get_service_descriptor_(ResourceServerT::service_type::service_full_name()); + Registry::register_resource_server_(API::get(), + std::make_shared(sd)); + } + + /// @brief Register resource client and server constructors + template + static void register_resource() { + register_resource_client(); + register_resource_server(); } - /// @brief Register an api. - /// @param api The api to be registered. - /// @param resource_registration `ResourceRegistration` with resource functionality. - static void register_resource(API api, - std::shared_ptr resource_registration); + /// @brief Lookup a registered server api. + /// @param api The api to lookup. + /// @return A `shared_ptr` to the registered api's `ResourceServerRegistration`. + static std::shared_ptr lookup_resource_server(API api); - /// @brief Lookup a registered api. + /// @brief Lookup a registered client api. /// @param api The api to lookup. - /// @return A `shared_ptr` to the registered api's `ResourceRegistration`. - static std::shared_ptr lookup_resource(API api); + /// @return A `shared_ptr` to the registered api's `ResourceClientRegistration`. + static std::shared_ptr lookup_resource_client(API api); /// @brief Provide information on registered resource models. /// @return A map from name to `ModelRegistration` of all registered resource models. - static const std::unordered_map>& + static const std::unordered_map>& registered_models(); /// @brief Provide access to registered resources. - /// @return A map from `API` to `ResourceRegistration` of all registered resources. - static const std::unordered_map>& - registered_resources(); + /// @return A map from `API` to `ResourceServerRegistration` of all registered resources. + static const std::unordered_map>& + registered_resource_servers(); /// @brief Initialized the Viam registry. No-op if it has already been called. static void initialize(); private: + static std::mutex lock_; static bool initialized_; + static std::unordered_map> resources_; + static std::unordered_map> client_apis_; + static std::unordered_map> server_apis_; + + static void register_resource_server_( + API api, std::shared_ptr resource_registration); + + static void register_resource_client_( + API api, std::shared_ptr resource_registration); static const google::protobuf::ServiceDescriptor* get_service_descriptor_( const char* service_full_name); - static std::unordered_map> resources_; - static std::unordered_map> apis_; + + static std::shared_ptr lookup_model_inlock_( + std::string name, const std::lock_guard&); }; } // namespace sdk diff --git a/src/viam/sdk/resource/resource.cpp b/src/viam/sdk/resource/resource.cpp index b548a0922..adbd6019f 100644 --- a/src/viam/sdk/resource/resource.cpp +++ b/src/viam/sdk/resource/resource.cpp @@ -21,7 +21,7 @@ std::string Resource::name() const { void Resource::reconfigure(Dependencies deps, ResourceConfig cfg){}; -ResourceName Resource::get_resource_name(std::string name) { +ResourceName Resource::get_resource_name(std::string name) const { ResourceName r; *r.mutable_namespace_() = kRDK; *r.mutable_type() = kResource; diff --git a/src/viam/sdk/resource/resource.hpp b/src/viam/sdk/resource/resource.hpp index 8bb9d808d..eacfd2126 100644 --- a/src/viam/sdk/resource/resource.hpp +++ b/src/viam/sdk/resource/resource.hpp @@ -23,7 +23,7 @@ class Resource { virtual API api() const = 0; /// @brief Returns a `ResourceName` for a particular resource name. - virtual viam::common::v1::ResourceName get_resource_name(std::string name); + virtual viam::common::v1::ResourceName get_resource_name(std::string name) const; /// @brief Reconfigures a resource. /// @param deps Dependencies of the resource. diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index a5e2165f9..23534f140 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -178,8 +178,8 @@ void RobotClient::refresh() { // TODO(RSDK-2066): as we create wrappers, make sure components in wrappers // are being properly registered from name.subtype(), or update what we're // using for lookup - const std::shared_ptr rs = - Registry::lookup_resource({name.namespace_(), name.type(), name.subtype()}); + const std::shared_ptr rs = + Registry::lookup_resource_client({name.namespace_(), name.type(), name.subtype()}); if (rs) { try { const std::shared_ptr rpc_client = diff --git a/src/viam/sdk/robot/service.cpp b/src/viam/sdk/robot/service.cpp index 5fabc88bd..fc2dac40e 100644 --- a/src/viam/sdk/robot/service.cpp +++ b/src/viam/sdk/robot/service.cpp @@ -54,7 +54,7 @@ std::vector RobotService_::generate_status(RepeatedPtrFieldresources()) { const std::shared_ptr resource = cmp.second; for (const auto& kv : Registry::registered_models()) { - const std::shared_ptr registration = kv.second; + const std::shared_ptr registration = kv.second; if (registration->api().resource_subtype() == resource->api().resource_subtype()) { bool resource_present = false; const ResourceName name = resource->get_resource_name(resource->name()); diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index bdb190b51..7988520c4 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -12,7 +12,7 @@ namespace sdk { Server::Server() : builder_(std::make_unique()) { Registry::initialize(); - for (const auto& rr : Registry::registered_resources()) { + for (const auto& rr : Registry::registered_resource_servers()) { auto new_manager = std::make_shared(); auto server = rr.second->create_resource_server(new_manager, *this); managed_servers_.emplace(rr.first, std::move(server)); diff --git a/src/viam/sdk/services/generic/client.hpp b/src/viam/sdk/services/generic/client.hpp index 4a8bf8e30..256d230be 100644 --- a/src/viam/sdk/services/generic/client.hpp +++ b/src/viam/sdk/services/generic/client.hpp @@ -18,6 +18,7 @@ namespace sdk { /// @ingroup GenericService class GenericServiceClient : public GenericService { public: + using interface_type = GenericService; GenericServiceClient(std::string name, std::shared_ptr channel); AttributeMap do_command(AttributeMap command) override; diff --git a/src/viam/sdk/services/generic/server.hpp b/src/viam/sdk/services/generic/server.hpp index f72ab2b94..0ce6f3149 100644 --- a/src/viam/sdk/services/generic/server.hpp +++ b/src/viam/sdk/services/generic/server.hpp @@ -8,6 +8,7 @@ #include #include +#include namespace viam { namespace sdk { @@ -18,6 +19,8 @@ namespace sdk { class GenericServiceServer : public ResourceServer, public viam::service::generic::v1::GenericService::Service { public: + using interface_type = GenericService; + using service_type = service::generic::v1::GenericService; explicit GenericServiceServer(std::shared_ptr manager); ::grpc::Status DoCommand(::grpc::ServerContext* context, diff --git a/src/viam/sdk/services/mlmodel/client.hpp b/src/viam/sdk/services/mlmodel/client.hpp index 437b37fd7..dd38f446f 100644 --- a/src/viam/sdk/services/mlmodel/client.hpp +++ b/src/viam/sdk/services/mlmodel/client.hpp @@ -30,6 +30,7 @@ namespace sdk { /// class MLModelServiceClient : public MLModelService { public: + using interface_type = MLModelService; using service_type = viam::service::mlmodel::v1::MLModelService; MLModelServiceClient(std::string name, std::shared_ptr channel); diff --git a/src/viam/sdk/services/mlmodel/server.hpp b/src/viam/sdk/services/mlmodel/server.hpp index e4ac1f8b8..108f65684 100644 --- a/src/viam/sdk/services/mlmodel/server.hpp +++ b/src/viam/sdk/services/mlmodel/server.hpp @@ -18,6 +18,7 @@ #include #include +#include namespace viam { namespace sdk { @@ -29,6 +30,8 @@ namespace sdk { class MLModelServiceServer : public ResourceServer, public ::viam::service::mlmodel::v1::MLModelService::Service { public: + using interface_type = MLModelService; + using service_type = service::mlmodel::v1::MLModelService; explicit MLModelServiceServer(std::shared_ptr manager); ::grpc::Status Infer(::grpc::ServerContext* context, diff --git a/src/viam/sdk/services/motion/client.hpp b/src/viam/sdk/services/motion/client.hpp index 19a0dddf2..11acd1ce0 100644 --- a/src/viam/sdk/services/motion/client.hpp +++ b/src/viam/sdk/services/motion/client.hpp @@ -17,6 +17,7 @@ namespace sdk { /// @ingroup Motion class MotionClient : public Motion { public: + using interface_type = Motion; MotionClient(std::string name, std::shared_ptr channel); bool move(const pose_in_frame& destination, const Name& component_name, diff --git a/src/viam/sdk/services/motion/server.hpp b/src/viam/sdk/services/motion/server.hpp index cc145f4fe..2796a1ecc 100644 --- a/src/viam/sdk/services/motion/server.hpp +++ b/src/viam/sdk/services/motion/server.hpp @@ -10,6 +10,7 @@ #include #include #include +#include namespace viam { namespace sdk { @@ -20,6 +21,8 @@ namespace sdk { class MotionServer : public ResourceServer, public viam::service::motion::v1::MotionService::Service { public: + using interface_type = Motion; + using service_type = service::motion::v1::MotionService; explicit MotionServer(std::shared_ptr manager); ::grpc::Status Move(::grpc::ServerContext* context, diff --git a/src/viam/sdk/services/service.cpp b/src/viam/sdk/services/service.cpp index 34976461a..29f6a429d 100644 --- a/src/viam/sdk/services/service.cpp +++ b/src/viam/sdk/services/service.cpp @@ -10,7 +10,7 @@ namespace viam { namespace sdk { -ResourceName Service::get_resource_name(std::string name) { +ResourceName Service::get_resource_name(std::string name) const { auto r = this->Resource::get_resource_name(name); *r.mutable_type() = kService; return r; diff --git a/src/viam/sdk/services/service.hpp b/src/viam/sdk/services/service.hpp index 101839666..817304867 100644 --- a/src/viam/sdk/services/service.hpp +++ b/src/viam/sdk/services/service.hpp @@ -11,7 +11,7 @@ namespace sdk { class Service : public Resource { public: - viam::common::v1::ResourceName get_resource_name(std::string name) override; + viam::common::v1::ResourceName get_resource_name(std::string name) const override; Service(); protected: diff --git a/src/viam/sdk/tests/mocks/mock_robot.cpp b/src/viam/sdk/tests/mocks/mock_robot.cpp index dfe6477d8..4c8f4055d 100644 --- a/src/viam/sdk/tests/mocks/mock_robot.cpp +++ b/src/viam/sdk/tests/mocks/mock_robot.cpp @@ -1,4 +1,3 @@ -#include "viam/sdk/common/proto_type.hpp" #include #include From a70d01bf7ad5a452f131a20d4cd9258521a6a8bd Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Thu, 8 Feb 2024 11:45:20 -0500 Subject: [PATCH 16/20] move comment --- src/viam/sdk/registry/registry.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index 5685e9c67..63821fabe 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -21,8 +21,6 @@ namespace viam { namespace sdk { -// TODO(RSDK-3030): instead of std::functions, consider making these functions -// virtual // TODO(RSDK-3030): one class per header class ResourceServerRegistration { public: @@ -61,6 +59,8 @@ class ResourceClientRegistration { ResourceClientRegistration() = default; }; +// TODO(RSDK-3030): instead of std::functions, consider making these functions +// virtual /// @class ModelRegistration /// @brief Information about a registered model, including a constructor and config validator. class ModelRegistration { From 80cdd4d178d8fdf45057c756b8112d3f3e51c986 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Thu, 8 Feb 2024 12:20:05 -0500 Subject: [PATCH 17/20] missing lock, a couple missing consts --- src/viam/sdk/registry/registry.cpp | 1 + src/viam/sdk/resource/resource_manager.cpp | 2 +- src/viam/sdk/resource/resource_manager.hpp | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index b5e4d6531..6ff5f6eeb 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -202,6 +202,7 @@ void register_resources() { } void Registry::initialize() { + const std::lock_guard lock(lock_); if (initialized_) { BOOST_LOG_TRIVIAL(warning) << "Attempted to initialize the Registry but it was already initialized."; diff --git a/src/viam/sdk/resource/resource_manager.cpp b/src/viam/sdk/resource/resource_manager.cpp index 201b82856..c013e770b 100644 --- a/src/viam/sdk/resource/resource_manager.cpp +++ b/src/viam/sdk/resource/resource_manager.cpp @@ -22,7 +22,7 @@ namespace viam { namespace sdk { -std::shared_ptr ResourceManager::resource(const std::string& name) { +std::shared_ptr ResourceManager::resource(const std::string& name) const { const std::lock_guard lock(lock_); if (resources_.find(name) != resources_.end()) { diff --git a/src/viam/sdk/resource/resource_manager.hpp b/src/viam/sdk/resource/resource_manager.hpp index c48a2f1b6..68c4bf21c 100644 --- a/src/viam/sdk/resource/resource_manager.hpp +++ b/src/viam/sdk/resource/resource_manager.hpp @@ -26,13 +26,13 @@ class ResourceManager { /// @brief Returns a resource. /// @param name the name of the desired resource. /// @throws `std::runtime_error` if the desired resource does not exist. - std::shared_ptr resource(const std::string& name); + std::shared_ptr resource(const std::string& name) const; /// @brief Returns a resource after dynamically downcasting to `T`. /// @param name of the desired resource. /// @throws `std::runtime_error` if the desired resource does not exist. template - std::shared_ptr resource(const std::string& name) { + std::shared_ptr resource(const std::string& name) const { static_assert(std::is_base_of::value, "T is not derived from Resource"); return std::dynamic_pointer_cast(resource(name)); } From 6b6e0e4b880e1a431fe276651b5a200f773b5070 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Thu, 8 Feb 2024 15:20:06 -0500 Subject: [PATCH 18/20] unconst method that messes with mutex --- src/viam/sdk/resource/resource_manager.cpp | 2 +- src/viam/sdk/resource/resource_manager.hpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/viam/sdk/resource/resource_manager.cpp b/src/viam/sdk/resource/resource_manager.cpp index c013e770b..201b82856 100644 --- a/src/viam/sdk/resource/resource_manager.cpp +++ b/src/viam/sdk/resource/resource_manager.cpp @@ -22,7 +22,7 @@ namespace viam { namespace sdk { -std::shared_ptr ResourceManager::resource(const std::string& name) const { +std::shared_ptr ResourceManager::resource(const std::string& name) { const std::lock_guard lock(lock_); if (resources_.find(name) != resources_.end()) { diff --git a/src/viam/sdk/resource/resource_manager.hpp b/src/viam/sdk/resource/resource_manager.hpp index 68c4bf21c..c48a2f1b6 100644 --- a/src/viam/sdk/resource/resource_manager.hpp +++ b/src/viam/sdk/resource/resource_manager.hpp @@ -26,13 +26,13 @@ class ResourceManager { /// @brief Returns a resource. /// @param name the name of the desired resource. /// @throws `std::runtime_error` if the desired resource does not exist. - std::shared_ptr resource(const std::string& name) const; + std::shared_ptr resource(const std::string& name); /// @brief Returns a resource after dynamically downcasting to `T`. /// @param name of the desired resource. /// @throws `std::runtime_error` if the desired resource does not exist. template - std::shared_ptr resource(const std::string& name) const { + std::shared_ptr resource(const std::string& name) { static_assert(std::is_base_of::value, "T is not derived from Resource"); return std::dynamic_pointer_cast(resource(name)); } From 188786ca57e1e6ab914a7770a27fb967f3faa93c Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Thu, 8 Feb 2024 15:23:56 -0500 Subject: [PATCH 19/20] fix ticket ##s --- src/viam/sdk/registry/registry.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index 63821fabe..9e7f5b284 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -21,7 +21,7 @@ namespace viam { namespace sdk { -// TODO(RSDK-3030): one class per header +// TODO(RSDK-6617): one class per header class ResourceServerRegistration { public: virtual ~ResourceServerRegistration(); @@ -59,7 +59,7 @@ class ResourceClientRegistration { ResourceClientRegistration() = default; }; -// TODO(RSDK-3030): instead of std::functions, consider making these functions +// TODO(RSDK-6616): instead of std::functions, consider making these functions // virtual /// @class ModelRegistration /// @brief Information about a registered model, including a constructor and config validator. From ad474471bc24946aeadaf12d49b870a1a7a0ecc4 Mon Sep 17 00:00:00 2001 From: Ethan Rodkin Date: Fri, 9 Feb 2024 15:09:57 -0500 Subject: [PATCH 20/20] final PR comments --- src/viam/sdk/registry/registry.cpp | 47 +++++++----------------------- src/viam/sdk/registry/registry.hpp | 13 +++++---- 2 files changed, 17 insertions(+), 43 deletions(-) diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 6ff5f6eeb..292401c53 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -11,59 +11,31 @@ #include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include #include #include -#include #include #include -#include #include #include -#include #include -#include #include #include -#include #include #include -#include #include #include -#include #include #include -#include #include #include -#include #include #include #include -#include #include #include #include -#include #include #include -#include #include #include #include @@ -91,7 +63,7 @@ void Registry::register_model(std::shared_ptr resource) throw std::runtime_error(err); } - resources_.emplace(reg_key, resource); + resources_.emplace(std::move(reg_key), std::move(resource)); } void Registry::register_resource_server_( @@ -113,7 +85,7 @@ void Registry::register_resource_client_( } std::shared_ptr Registry::lookup_model_inlock_( - std::string name, const std::lock_guard&) { + const std::string& name, const std::lock_guard&) { if (resources_.find(name) == resources_.end()) { return nullptr; } @@ -121,18 +93,19 @@ std::shared_ptr Registry::lookup_model_inlock_( return resources_.at(name); } -std::shared_ptr Registry::lookup_model(std::string name) { +std::shared_ptr Registry::lookup_model(const std::string& name) { const std::lock_guard lock(lock_); - return lookup_model_inlock_(name, lock); + return lookup_model_inlock_(name, std::move(lock)); } -std::shared_ptr Registry::lookup_model(API api, Model model) { +std::shared_ptr Registry::lookup_model(const API& api, + const Model& model) { const std::lock_guard lock(lock_); const std::string name = api.to_string() + "/" + model.to_string(); - return lookup_model_inlock_(name, lock); + return lookup_model_inlock_(std::move(name), std::move(lock)); } -std::shared_ptr Registry::lookup_resource_server(API api) { +std::shared_ptr Registry::lookup_resource_server(const API& api) { const std::lock_guard lock(lock_); if (server_apis_.find(api) == server_apis_.end()) { return nullptr; @@ -141,7 +114,7 @@ std::shared_ptr Registry::lookup_resource_serv return server_apis_.at(api); } -std::shared_ptr Registry::lookup_resource_client(API api) { +std::shared_ptr Registry::lookup_resource_client(const API& api) { const std::lock_guard lock(lock_); if (client_apis_.find(api) == client_apis_.end()) { return nullptr; @@ -171,7 +144,7 @@ Registry::registered_models() { } // NOLINTNEXTLINE(readability-convert-member-functions-to-static) -Status ModelRegistration::create_status(std::shared_ptr resource) const { +Status ModelRegistration::create_status(const std::shared_ptr& resource) const { Status status; *status.mutable_name() = resource->get_resource_name(resource->name()); *status.mutable_status() = google::protobuf::Struct(); diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index 9e7f5b284..89b4ff931 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -96,7 +96,7 @@ class ModelRegistration { std::function(ResourceConfig)> validate; /// @brief Creates a `Status` object for a given resource. - viam::robot::v1::Status create_status(std::shared_ptr resource) const; + viam::robot::v1::Status create_status(const std::shared_ptr& resource) const; private: // default_validator is the default validator for all models if no validator @@ -120,13 +120,14 @@ class Registry { /// @brief Lookup a given registered resource. /// @param name The name of the resource to lookup. /// @return a `shared_ptr` to the resource's registration data. - static std::shared_ptr lookup_model(std::string name); + static std::shared_ptr lookup_model(const std::string& name); /// @brief Lookup a given registered resource. /// @param api The api of the resource to lookup. /// @param model The model of the resource to lookup. /// @return a `shared_ptr` to the resource's registration data. - static std::shared_ptr lookup_model(API api, Model model); + static std::shared_ptr lookup_model(const API& api, + const Model& model); /// @brief Register a resource client constructor template @@ -175,12 +176,12 @@ class Registry { /// @brief Lookup a registered server api. /// @param api The api to lookup. /// @return A `shared_ptr` to the registered api's `ResourceServerRegistration`. - static std::shared_ptr lookup_resource_server(API api); + static std::shared_ptr lookup_resource_server(const API& api); /// @brief Lookup a registered client api. /// @param api The api to lookup. /// @return A `shared_ptr` to the registered api's `ResourceClientRegistration`. - static std::shared_ptr lookup_resource_client(API api); + static std::shared_ptr lookup_resource_client(const API& api); /// @brief Provide information on registered resource models. /// @return A map from name to `ModelRegistration` of all registered resource models. @@ -212,7 +213,7 @@ class Registry { const char* service_full_name); static std::shared_ptr lookup_model_inlock_( - std::string name, const std::lock_guard&); + const std::string& name, const std::lock_guard&); }; } // namespace sdk