From 31437c36d4fd2d8db05685d5511e6f1588cac828 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 18 Feb 2025 11:02:17 -0500 Subject: [PATCH 01/31] initial commit of instance object --- src/viam/sdk/CMakeLists.txt | 2 ++ src/viam/sdk/common/instance.cpp | 11 +++++++++++ src/viam/sdk/common/instance.hpp | 24 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 src/viam/sdk/common/instance.cpp create mode 100644 src/viam/sdk/common/instance.hpp diff --git a/src/viam/sdk/CMakeLists.txt b/src/viam/sdk/CMakeLists.txt index cd7992f2a..e42db9ae5 100644 --- a/src/viam/sdk/CMakeLists.txt +++ b/src/viam/sdk/CMakeLists.txt @@ -50,6 +50,7 @@ target_sources(viamsdk PRIVATE common/client_helper.cpp common/exception.cpp + common/instance.cpp common/linear_algebra.cpp common/pose.cpp common/proto_value.cpp @@ -142,6 +143,7 @@ target_sources(viamsdk FILES ../../viam/sdk/common/client_helper.hpp ../../viam/sdk/common/exception.hpp + ../../viam/sdk/common/instance.hpp ../../viam/sdk/common/linear_algebra.hpp ../../viam/sdk/common/pose.hpp ../../viam/sdk/common/proto_convert.hpp diff --git a/src/viam/sdk/common/instance.cpp b/src/viam/sdk/common/instance.cpp new file mode 100644 index 000000000..1ec93d01f --- /dev/null +++ b/src/viam/sdk/common/instance.cpp @@ -0,0 +1,11 @@ +#include + +namespace viam { +namespace sdk { + +Instance::Instance() { + registry_.initialize(); +} + +} // namespace sdk +} // namespace viam diff --git a/src/viam/sdk/common/instance.hpp b/src/viam/sdk/common/instance.hpp new file mode 100644 index 000000000..8297b74e4 --- /dev/null +++ b/src/viam/sdk/common/instance.hpp @@ -0,0 +1,24 @@ +#pragma once + +#include + +namespace viam { +namespace sdk { + +/// @brief Instance management for Viam C++ SDK applications. +/// This is a single instance class which is responsible for global setup and teardown related to +/// the SDK. An Instance must be constructed before doing anything else in a program, and it must +/// remain alive in a valid state for the duration of the program. Creating multiple overlapping +/// Instance objects is an error. +class Instance { + public: + Instance(); + + Registry& registry(); + + private: + Registry registry_; +}; + +} // namespace sdk +} // namespace viam From 932ebaa1885b2e238cf339d0642c681ebf653b7a Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 18 Feb 2025 11:04:40 -0500 Subject: [PATCH 02/31] wip: initial commit to get sdk building with instance --- src/viam/sdk/module/service.cpp | 38 ++++++++++-------- src/viam/sdk/module/service.hpp | 7 +++- src/viam/sdk/registry/registry.cpp | 62 ++++++++++++++---------------- src/viam/sdk/registry/registry.hpp | 51 ++++++++++++------------ src/viam/sdk/robot/client.cpp | 27 +++++++------ src/viam/sdk/robot/client.hpp | 21 ++++++++-- src/viam/sdk/rpc/server.cpp | 5 +-- src/viam/sdk/rpc/server.hpp | 4 +- 8 files changed, 119 insertions(+), 96 deletions(-) diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 53f1fe9bc..526beecb5 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -57,7 +57,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { std::shared_ptr res; const Dependencies deps = parent.get_dependencies_(&request->dependencies(), cfg.name()); const std::shared_ptr reg = - Registry::lookup_model(cfg.api(), cfg.model()); + parent.registry_.lookup_model(cfg.api(), cfg.model()); if (reg) { try { res = reg->construct_resource(deps, cfg); @@ -111,7 +111,8 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { 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 = + parent.registry_.lookup_model(cfg.name()); if (reg) { try { const std::shared_ptr res = reg->construct_resource(deps, cfg); @@ -131,7 +132,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { ResourceConfig cfg = from_proto(proto); const std::shared_ptr reg = - Registry::lookup_model(cfg.api(), cfg.model()); + parent.registry_.lookup_model(cfg.api(), cfg.model()); if (!reg) { return grpc::Status(grpc::UNKNOWN, "unable to validate resource " + cfg.resource_name().name() + @@ -207,31 +208,36 @@ Dependencies ModuleService::get_dependencies_( std::shared_ptr ModuleService::get_parent_resource_(const Name& name) { if (!parent_) { - parent_ = RobotClient::at_local_socket(parent_addr_, {0, boost::none}); + parent_ = RobotClient::at_local_socket(parent_addr_, {0, boost::none}, registry_); } return parent_->resource_by_name(name); } -ModuleService::ModuleService(std::string addr) - : module_(std::make_unique(std::move(addr))), server_(std::make_unique()) { +ModuleService::ModuleService(std::string addr, Registry& registry) + : registry_(registry), + module_(std::make_unique(std::move(addr))), + server_(std::make_unique(®istry_)) { impl_ = std::make_unique(*this); } ModuleService::ModuleService(int argc, char** argv, - const std::vector>& registrations) - : ModuleService([argc, argv] { - if (argc < 2) { - throw Exception(ErrorCondition::k_connection, - "Need socket path as command line argument"); - } - return argv[1]; - }()) { + const std::vector>& registrations, + Registry& registry) + : ModuleService( + [argc, argv] { + if (argc < 2) { + throw Exception(ErrorCondition::k_connection, + "Need socket path as command line argument"); + } + return argv[1]; + }(), + registry) { set_logger_severity_from_args(argc, argv); for (auto&& mr : registrations) { - Registry::register_model(mr); + registry_.register_model(mr); add_model_from_registry(mr->api(), mr->model()); } } @@ -274,7 +280,7 @@ void ModuleService::add_model_from_registry_inlock_(API api, Model model, const std::lock_guard&) { const std::shared_ptr creator = - Registry::lookup_resource_server(api); + registry_.lookup_resource_server(api); std::string name; if (creator && creator->service_descriptor()) { name = creator->service_descriptor()->full_name(); diff --git a/src/viam/sdk/module/service.hpp b/src/viam/sdk/module/service.hpp index 799ba1ab5..cffc47fe7 100644 --- a/src/viam/sdk/module/service.hpp +++ b/src/viam/sdk/module/service.hpp @@ -31,7 +31,7 @@ class ModuleService { public: /// @brief Creates a new ModuleService that can serve on the provided socket. /// @param addr Address of socket to serve on. - explicit ModuleService(std::string addr); + explicit ModuleService(std::string addr, Registry& registry); /// @brief Creates a new ModuleService. Socket path and log level will be /// inferred from passed in command line arguments, and passed in model @@ -41,7 +41,8 @@ class ModuleService { /// @param registrations Models to register and add to the module. explicit ModuleService(int argc, char** argv, - const std::vector>& registrations); + const std::vector>& registrations, + Registry& registry); ~ModuleService(); /// @brief Starts module. serve will return when SIGINT or SIGTERM is received @@ -64,6 +65,8 @@ class ModuleService { std::string const& resource_name); std::shared_ptr get_parent_resource_(const Name& name); + Registry& registry_; + std::mutex lock_; std::unique_ptr module_; std::shared_ptr parent_; diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 57e857f00..71e3e7711 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -122,7 +122,7 @@ void Registry::register_resource_client_( } std::shared_ptr Registry::lookup_model_inlock_( - const std::string& name, const std::lock_guard&) { + const std::string& name, const std::lock_guard&) const { if (resources_.find(name) == resources_.end()) { return nullptr; } @@ -130,19 +130,20 @@ std::shared_ptr Registry::lookup_model_inlock_( return resources_.at(name); } -std::shared_ptr Registry::lookup_model(const std::string& name) { +std::shared_ptr Registry::lookup_model(const std::string& name) const { const std::lock_guard lock(lock_); return lookup_model_inlock_(name, lock); } std::shared_ptr Registry::lookup_model(const API& api, - const Model& model) { + const Model& model) const { const std::lock_guard lock(lock_); const std::string name = api.to_string() + "/" + model.to_string(); return lookup_model_inlock_(name, lock); } -std::shared_ptr Registry::lookup_resource_server(const API& api) { +std::shared_ptr Registry::lookup_resource_server( + const API& api) const { const std::lock_guard lock(lock_); if (server_apis_.find(api) == server_apis_.end()) { return nullptr; @@ -151,7 +152,8 @@ std::shared_ptr Registry::lookup_resource_serv return server_apis_.at(api); } -std::shared_ptr Registry::lookup_resource_client(const API& api) { +std::shared_ptr Registry::lookup_resource_client( + const API& api) const { const std::lock_guard lock(lock_); if (client_apis_.find(api) == client_apis_.end()) { return nullptr; @@ -161,7 +163,7 @@ std::shared_ptr Registry::lookup_resource_clie } const google::protobuf::ServiceDescriptor* Registry::get_service_descriptor_( - const char* service_full_name) { + const char* service_full_name) const { const google::protobuf::DescriptorPool* p = google::protobuf::DescriptorPool::generated_pool(); const google::protobuf::ServiceDescriptor* sd = p->FindServiceByName(service_full_name); if (!sd) { @@ -171,12 +173,12 @@ const google::protobuf::ServiceDescriptor* Registry::get_service_descriptor_( } const std::unordered_map>& -Registry::registered_resource_servers() { +Registry::registered_resource_servers() const { return server_apis_; } const std::unordered_map>& -Registry::registered_models() { +Registry::registered_models() const { return resources_; } @@ -184,28 +186,28 @@ const google::protobuf::ServiceDescriptor* ResourceServerRegistration::service_d return service_descriptor_; } -void register_resources() { +void Registry::register_resources() { // Register all components - 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(); - Registry::register_resource(); - Registry::register_resource(); - Registry::register_resource(); - Registry::register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); // Register all services - Registry::register_resource(); - Registry::register_resource(); - Registry::register_resource(); - Registry::register_resource(); + register_resource(); + register_resource(); + register_resource(); + register_resource(); } void Registry::initialize() { @@ -220,11 +222,5 @@ void Registry::initialize() { grpc::reflection::InitProtoReflectionServerBuilderPlugin(); } -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 } // namespace viam diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index b00d98833..4431575d6 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -106,23 +106,22 @@ class Registry { /// @brief Registers a resource with the Registry. /// @param resource An object containing resource registration information. /// @throws `Exception` if the resource has already been registered. - static void register_model(std::shared_ptr resource); + 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(const std::string& name); + std::shared_ptr lookup_model(const std::string& name) const; /// @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(const API& api, - const Model& model); + std::shared_ptr lookup_model(const API& api, const Model& model) const; /// @brief Register a resource client constructor template - static void register_resource_client() { + void register_resource_client() { class ResourceClientRegistration2 final : public ResourceClientRegistration { public: using ResourceClientRegistration::ResourceClientRegistration; @@ -139,7 +138,7 @@ class Registry { /// @brief Register a resource server constructor. template - static void register_resource_server() { + void register_resource_server() { class ResourceServerRegistration2 final : public ResourceServerRegistration { public: using ResourceServerRegistration::ResourceServerRegistration; @@ -159,7 +158,7 @@ class Registry { /// @brief Register resource client and server constructors template - static void register_resource() { + void register_resource() { register_resource_client(); register_resource_server(); } @@ -167,44 +166,46 @@ 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(const API& api); + std::shared_ptr lookup_resource_server(const API& api) const; /// @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(const API& api); + std::shared_ptr lookup_resource_client(const API& api) const; /// @brief Provide information on registered resource models. /// @return A map from name to `ModelRegistration` of all registered resource models. - static const std::unordered_map>& - registered_models(); + const std::unordered_map>& + registered_models() const; /// @brief Provide access to registered resources. /// @return A map from `API` to `ResourceServerRegistration` of all registered resources. - static const std::unordered_map>& - registered_resource_servers(); + const std::unordered_map>& + registered_resource_servers() const; /// @brief Initialized the Viam registry. No-op if it has already been called. - static void initialize(); + 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_; + mutable std::mutex lock_; + bool initialized_{false}; + std::unordered_map> resources_; + std::unordered_map> client_apis_; + std::unordered_map> server_apis_; - static void register_resource_server_( + void register_resources(); + + void register_resource_server_( API api, std::shared_ptr resource_registration); - static void register_resource_client_( + void register_resource_client_( API api, std::shared_ptr resource_registration); - static const google::protobuf::ServiceDescriptor* get_service_descriptor_( - const char* service_full_name); + const google::protobuf::ServiceDescriptor* get_service_descriptor_( + const char* service_full_name) const; - static std::shared_ptr lookup_model_inlock_( - const std::string& name, const std::lock_guard&); + std::shared_ptr lookup_model_inlock_( + const std::string& name, const std::lock_guard&) const; }; } // namespace sdk diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index fff691fd2..6ac049eba 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -178,7 +178,7 @@ void RobotClient::refresh() { // are being properly registered from name.subtype(), or update what we're // using for lookup const std::shared_ptr rs = - Registry::lookup_resource_client({name.namespace_(), name.type(), name.subtype()}); + registry_.lookup_resource_client({name.namespace_(), name.type(), name.subtype()}); if (rs) { try { const std::shared_ptr rpc_client = @@ -221,13 +221,12 @@ void RobotClient::refresh_every() { } }; -RobotClient::RobotClient(std::shared_ptr channel) - : channel_(channel->channel()), +RobotClient::RobotClient(std::shared_ptr channel, Registry& registry) + : registry_(registry), + channel_(channel->channel()), viam_channel_(std::move(channel)), should_close_channel_(false), - impl_(std::make_unique(RobotService::NewStub(channel_))) { - Registry::initialize(); -} + impl_(std::make_unique(RobotService::NewStub(channel_))) {} std::vector RobotClient::resource_names() const { const std::lock_guard lock(lock_); @@ -235,8 +234,10 @@ std::vector RobotClient::resource_names() const { } std::shared_ptr RobotClient::with_channel(std::shared_ptr channel, - const Options& options) { - std::shared_ptr robot = std::make_shared(std::move(channel)); + const Options& options, + Registry& registry) { + std::shared_ptr robot = + std::make_shared(std::move(channel), registry); robot->refresh_interval_ = options.refresh_interval(); robot->should_refresh_ = (robot->refresh_interval_ > 0); if (robot->should_refresh_) { @@ -254,23 +255,25 @@ std::shared_ptr RobotClient::with_channel(std::shared_ptr RobotClient::at_address(const std::string& address, - const Options& options) { + const Options& options, + Registry& registry) { const char* uri = address.c_str(); auto channel = ViamChannel::dial(uri, options.dial_options()); - std::shared_ptr robot = RobotClient::with_channel(channel, options); + std::shared_ptr robot = RobotClient::with_channel(channel, options, registry); robot->should_close_channel_ = true; return robot; }; std::shared_ptr RobotClient::at_local_socket(const std::string& address, - const Options& options) { + const Options& options, + Registry& registry) { const std::string addr = "unix://" + address; const char* uri = addr.c_str(); const std::shared_ptr channel = sdk::impl::create_viam_channel(uri, grpc::InsecureChannelCredentials()); auto viam_channel = std::make_shared(channel, address.c_str(), nullptr); - std::shared_ptr robot = RobotClient::with_channel(viam_channel, options); + std::shared_ptr robot = RobotClient::with_channel(viam_channel, options, registry); robot->should_close_channel_ = true; return robot; diff --git a/src/viam/sdk/robot/client.hpp b/src/viam/sdk/robot/client.hpp index 9d63bd14b..09834e1d8 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -68,7 +68,8 @@ class RobotClient { /// @param address The address of the robot (IP address, URI, URL, etc.) /// @param options Options for connecting and refreshing. static std::shared_ptr at_address(const std::string& address, - const Options& options); + const Options& options, + Registry& registry); /// @brief Creates a robot client connected to the robot at the provided local socket. /// @param address The local socket of the robot (a .sock file, etc.). @@ -76,7 +77,8 @@ class RobotClient { /// Creates a direct connection to the robot using the `unix://` scheme. /// Only useful for connecting to robots across Unix sockets. static std::shared_ptr at_local_socket(const std::string& address, - const Options& options); + const Options& options, + Registry& registry); /// @brief Creates a robot client connected to the provided channel. /// @param channel The channel to connect with. @@ -84,8 +86,11 @@ class RobotClient { /// Connects directly to a pre-existing channel. A robot created this way must be /// `close()`d manually. static std::shared_ptr with_channel(std::shared_ptr channel, - const Options& options); - RobotClient(std::shared_ptr channel); + const Options& options, + Registry& registry); + + RobotClient(std::shared_ptr channel, Registry& registry); + std::vector resource_names() const; /// @brief Lookup and return a `shared_ptr` to a resource. @@ -147,17 +152,25 @@ class RobotClient { status get_machine_status() const; private: + Registry& registry_; + std::vector> threads_; + std::atomic should_refresh_; unsigned int refresh_interval_; + std::shared_ptr channel_; std::shared_ptr viam_channel_; bool should_close_channel_; + struct impl; std::unique_ptr impl_; + mutable std::mutex lock_; + std::vector resource_names_; ResourceManager resource_manager_; + void refresh_every(); }; diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index 408f23bf6..b2c46c695 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -15,12 +15,11 @@ namespace viam { namespace sdk { -Server::Server() : builder_(std::make_unique()) { +Server::Server(const Registry* registry) : builder_(std::make_unique()) { builder_->SetMaxReceiveMessageSize(kMaxMessageSize); builder_->SetMaxSendMessageSize(kMaxMessageSize); builder_->SetMaxMessageSize(kMaxMessageSize); - Registry::initialize(); - for (const auto& rr : Registry::registered_resource_servers()) { + 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/rpc/server.hpp b/src/viam/sdk/rpc/server.hpp index e735a6351..5f868b779 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -23,11 +23,13 @@ class TestServer; namespace sdk { +class Registry; + /// @class Server server.hpp "rpc/server.hpp" /// @brief Defines gRPC `Server` functionality. class Server { public: - Server(); + Server(const Registry* registry); ~Server(); /// @brief Starts the grpc server. Can only be called once. From 04d0158412c31836d226576668c90ce36234d4f0 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 18 Feb 2025 14:14:58 -0500 Subject: [PATCH 03/31] add missing implementation --- src/viam/sdk/common/instance.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/viam/sdk/common/instance.cpp b/src/viam/sdk/common/instance.cpp index 1ec93d01f..3c277273d 100644 --- a/src/viam/sdk/common/instance.cpp +++ b/src/viam/sdk/common/instance.cpp @@ -7,5 +7,9 @@ Instance::Instance() { registry_.initialize(); } +Registry& Instance::registry() { + return registry_; +} + } // namespace sdk } // namespace viam From a105cef7426252039a7917153e0a34d3afc0a5a2 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 18 Feb 2025 14:15:56 -0500 Subject: [PATCH 04/31] update examples to use instance --- src/viam/examples/modules/simple/client.cpp | 5 ++++- src/viam/examples/modules/simple/main.cpp | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/viam/examples/modules/simple/client.cpp b/src/viam/examples/modules/simple/client.cpp index fcaab5eff..ac014d1e7 100644 --- a/src/viam/examples/modules/simple/client.cpp +++ b/src/viam/examples/modules/simple/client.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -10,6 +11,8 @@ using namespace viam::sdk; int main() { + Instance inst; + const char* uri = "http://localhost:8080/"; // replace with your URI if connecting securely DialOptions dial_options; dial_options.set_allow_insecure_downgrade(true); // set to false if connecting securely @@ -24,7 +27,7 @@ int main() { std::string address(uri); Options options(1, opts); - std::shared_ptr robot = RobotClient::at_address(address, options); + std::shared_ptr robot = RobotClient::at_address(address, options, inst.registry()); // Print resources std::cout << "Resources\n"; diff --git a/src/viam/examples/modules/simple/main.cpp b/src/viam/examples/modules/simple/main.cpp index cfd783f66..b899d3b44 100644 --- a/src/viam/examples/modules/simple/main.cpp +++ b/src/viam/examples/modules/simple/main.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -75,6 +76,8 @@ ProtoStruct MySensor::get_readings(const ProtoStruct&) { } int main(int argc, char** argv) try { + Instance inst; + Model mysensor_model("viam", "sensor", "mysensor"); std::shared_ptr mr = std::make_shared( @@ -84,7 +87,7 @@ int main(int argc, char** argv) try { &MySensor::validate); std::vector> mrs = {mr}; - auto my_mod = std::make_shared(argc, argv, mrs); + auto my_mod = std::make_shared(argc, argv, mrs, inst.registry()); my_mod->serve(); return EXIT_SUCCESS; From 403f31610d1b95a705b466fad0b0873934acec0c Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 18 Feb 2025 16:21:21 -0500 Subject: [PATCH 05/31] only instance can construct registry --- src/viam/sdk/registry/registry.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index 4431575d6..a98e05c6e 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -187,6 +187,10 @@ class Registry { void initialize(); private: + friend class Instance; + + Registry() = default; + mutable std::mutex lock_; bool initialized_{false}; std::unordered_map> resources_; From 27eeb1331c28810243859a7c5f0f5606a4e69ba6 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Fri, 21 Feb 2025 12:55:42 -0500 Subject: [PATCH 06/31] make tests work with registry member approach --- src/viam/sdk/registry/registry.hpp | 6 +++--- src/viam/sdk/tests/mocks/mock_robot.cpp | 2 +- src/viam/sdk/tests/test_robot.cpp | 19 +++++++++++-------- src/viam/sdk/tests/test_utils.hpp | 17 +++++++++++++++-- 4 files changed, 30 insertions(+), 14 deletions(-) diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index a98e05c6e..75e2324e3 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -183,14 +183,14 @@ class Registry { const std::unordered_map>& registered_resource_servers() const; - /// @brief Initialized the Viam registry. No-op if it has already been called. - void initialize(); - private: friend class Instance; Registry() = default; + /// @brief Initialized the Viam registry. No-op if it has already been called. + void initialize(); + mutable std::mutex lock_; bool initialized_{false}; std::unordered_map> resources_; diff --git a/src/viam/sdk/tests/mocks/mock_robot.cpp b/src/viam/sdk/tests/mocks/mock_robot.cpp index cdef0fe78..8e28b944b 100644 --- a/src/viam/sdk/tests/mocks/mock_robot.cpp +++ b/src/viam/sdk/tests/mocks/mock_robot.cpp @@ -26,7 +26,7 @@ std::vector registered_models_for_resource(const std::shared_ptr std::string resource_type; std::string resource_subtype; std::vector resource_names; - for (const auto& kv : Registry::registered_models()) { + for (const auto& kv : GlobalInstance::registry().registered_models()) { const std::shared_ptr reg = kv.second; if (reg->api() == resource->api()) { resource_type = reg->api().resource_type(); diff --git a/src/viam/sdk/tests/test_robot.cpp b/src/viam/sdk/tests/test_robot.cpp index 9f1093d49..e7ccc0de1 100644 --- a/src/viam/sdk/tests/test_robot.cpp +++ b/src/viam/sdk/tests/test_robot.cpp @@ -42,7 +42,7 @@ void robot_client_to_mocks_pipeline(F&& test_case) { rm->add(std::string("mock_generic"), generic::MockGenericComponent::get_mock_generic()); 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(); + auto server = std::make_shared(&GlobalInstance::registry()); MockRobotService service(rm, *server); server->start(); @@ -51,7 +51,8 @@ void robot_client_to_mocks_pipeline(F&& test_case) { auto test_server = TestServer(server); auto grpc_channel = test_server.grpc_in_process_channel(); auto viam_channel = std::make_shared(grpc_channel, "", nullptr); - auto client = RobotClient::with_channel(viam_channel, Options(0, boost::none)); + auto client = RobotClient::with_channel( + viam_channel, Options(0, boost::none), GlobalInstance::registry()); // Run the passed-in test case on the created stack and give access to the // created RobotClient and MockRobotService. @@ -62,6 +63,8 @@ void robot_client_to_mocks_pipeline(F&& test_case) { } BOOST_AUTO_TEST_CASE(test_registering_resources) { + auto& registry = GlobalInstance::registry(); + // To test with mock resources we need to be able to create them, which means registering // constructors. This tests that we register correctly. Model camera_model("fake", "fake", "mock_camera"); @@ -70,7 +73,7 @@ BOOST_AUTO_TEST_CASE(test_registering_resources) { camera_model, [](Dependencies, ResourceConfig cfg) { return camera::MockCamera::get_mock_camera(); }, [](ResourceConfig cfg) -> std::vector { return {}; }); - Registry::register_model(cr); + registry.register_model(cr); Model generic_model("fake", "fake", "mock_generic"); std::shared_ptr gr = std::make_shared( @@ -80,7 +83,7 @@ BOOST_AUTO_TEST_CASE(test_registering_resources) { return generic::MockGenericComponent::get_mock_generic(); }, [](ResourceConfig cfg) -> std::vector { return {}; }); - Registry::register_model(gr); + registry.register_model(gr); Model motor_model("fake", "fake", "mock_motor"); std::shared_ptr mr = std::make_shared( @@ -88,11 +91,11 @@ BOOST_AUTO_TEST_CASE(test_registering_resources) { motor_model, [](Dependencies, ResourceConfig cfg) { return motor::MockMotor::get_mock_motor(); }, [](ResourceConfig cfg) -> std::vector { return {}; }); - Registry::register_model(mr); + registry.register_model(mr); - BOOST_CHECK(Registry::lookup_model(API::get(), camera_model)); - BOOST_CHECK(Registry::lookup_model(API::get(), generic_model)); - BOOST_CHECK(Registry::lookup_model(API::get(), motor_model)); + BOOST_CHECK(registry.lookup_model(API::get(), camera_model)); + BOOST_CHECK(registry.lookup_model(API::get(), generic_model)); + BOOST_CHECK(registry.lookup_model(API::get(), motor_model)); } BOOST_AUTO_TEST_CASE(test_resource_names) { diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index 400bc54a2..c24bed482 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -2,6 +2,7 @@ #include +#include #include #include #include @@ -10,6 +11,17 @@ namespace viam { namespace sdktests { +struct GlobalInstance { + static sdk::Instance& get() { + static sdk::Instance inst; + return inst; + } + + static sdk::Registry& registry() { + return get().registry(); + } +}; + using namespace viam::sdk; ProtoStruct fake_map(); @@ -49,7 +61,7 @@ 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) { - auto server = std::make_shared(); + auto server = std::make_shared(&GlobalInstance::registry()); // normally the high level server service (either robot or module) handles adding managed // resources, but in this case we must do it ourselves. @@ -61,7 +73,8 @@ void client_to_mock_pipeline(std::shared_ptr mock, F&& test_case) { auto test_server = TestServer(server); auto grpc_channel = test_server.grpc_in_process_channel(); - auto resource_client = Registry::lookup_resource_client(API::get()) + auto resource_client = GlobalInstance::registry() + .lookup_resource_client(API::get()) ->create_rpc_client(mock->name(), std::move(grpc_channel)); // Run the passed-in test case on the created stack and give access to the From ce2aa8c54852cb2e88ff53dbc831331dd0081e02 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Mon, 24 Feb 2025 16:17:18 -0500 Subject: [PATCH 07/31] pimpl registry and update examples --- src/viam/examples/camera/example_camera.cpp | 8 +++++++- src/viam/examples/dial/example_dial.cpp | 7 ++++++- .../dial_api_key/example_dial_api_key.cpp | 7 ++++++- .../example_audio_classification_client.cpp | 9 +++++++-- src/viam/examples/modules/complex/client.cpp | 11 ++++++++--- src/viam/examples/modules/complex/main.cpp | 11 ++++++++--- .../modules/complex/test_complex_module.cpp | 5 +++-- src/viam/examples/motor/example_motor.cpp | 8 +++++++- src/viam/sdk/common/instance.cpp | 16 ++++++++++++---- src/viam/sdk/common/instance.hpp | 10 +++++++--- src/viam/sdk/module/service.cpp | 16 ++++++++-------- src/viam/sdk/module/service.hpp | 6 +++--- src/viam/sdk/registry/registry.hpp | 1 - src/viam/sdk/robot/client.cpp | 10 +++++----- src/viam/sdk/robot/client.hpp | 14 ++++++++------ src/viam/sdk/tests/mocks/mock_robot.cpp | 2 +- src/viam/sdk/tests/test_robot.cpp | 4 ++-- src/viam/sdk/tests/test_utils.hpp | 6 +++--- 18 files changed, 101 insertions(+), 50 deletions(-) diff --git a/src/viam/examples/camera/example_camera.cpp b/src/viam/examples/camera/example_camera.cpp index bc1b0ab9a..b0202a27e 100644 --- a/src/viam/examples/camera/example_camera.cpp +++ b/src/viam/examples/camera/example_camera.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -13,6 +14,11 @@ int main() { using std::endl; namespace vs = ::viam::sdk; try { + // Every Viam C++ SDK program must have one and only one Instance object which is created + // before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + vs::Instance inst; + // 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"); @@ -35,7 +41,7 @@ int main() { std::shared_ptr robot; try { - robot = vs::RobotClient::at_address(robot_address, options); + robot = vs::RobotClient::at_address(robot_address, options, inst.registry()); cout << "Successfully connected to the robot" << endl; } catch (const std::exception& e) { cerr << "Failed to connect to the robot. Exiting." << endl; diff --git a/src/viam/examples/dial/example_dial.cpp b/src/viam/examples/dial/example_dial.cpp index efa783dfb..28ccd7659 100644 --- a/src/viam/examples/dial/example_dial.cpp +++ b/src/viam/examples/dial/example_dial.cpp @@ -9,6 +9,7 @@ #include +#include #include #include #include @@ -16,6 +17,10 @@ using namespace viam::sdk; int main() { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + Instance inst; + const char* uri = ""; DialOptions dial_options; std::string type = ""; @@ -27,7 +32,7 @@ int main() { Options options(1, opts); // connect to robot, ensure we can refresh it - std::shared_ptr robot = RobotClient::at_address(address, options); + std::shared_ptr robot = RobotClient::at_address(address, options, inst.registry()); // ensure we can query resources std::vector resource_names = robot->resource_names(); 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 4c47beffe..7c9e82fef 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 @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -18,6 +19,10 @@ using namespace viam::sdk; namespace po = boost::program_options; int main(int argc, char* argv[]) { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + Instance inst; + po::options_description desc("Allowed options"); desc.add_options()("help", "List options and exit")( "uri", po::value(), "URI of robot")( @@ -41,7 +46,7 @@ int main(int argc, char* argv[]) { // connect to robot, ensure we can refresh it std::shared_ptr robot = - RobotClient::at_address(vm["uri"].as(), options); + RobotClient::at_address(vm["uri"].as(), options, inst.registry()); // ensure we can query resources std::vector resource_names = robot->resource_names(); diff --git a/src/viam/examples/mlmodel/example_audio_classification_client.cpp b/src/viam/examples/mlmodel/example_audio_classification_client.cpp index 95e26f57c..ab06a082e 100644 --- a/src/viam/examples/mlmodel/example_audio_classification_client.cpp +++ b/src/viam/examples/mlmodel/example_audio_classification_client.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include @@ -78,6 +79,10 @@ constexpr char kRobotConfigTemplate[] = R"( } // namespace int main(int argc, char* argv[]) try { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + viam::sdk::Instance inst; + // Build up our command line options. The example operates in two // modes. In the "--generate" mode, it takes command line // parameters needed to satisfy the interpolation points in the @@ -234,8 +239,8 @@ int main(int argc, char* argv[]) try { dial_options.set_entity(opt_api_key_id.get()); dial_options.set_credentials(viam::sdk::Credentials("api-key", opt_api_key.get())); - auto robot = - vsdk::RobotClient::at_address(opt_robot_host.get(), {0, {std::move(dial_options)}}); + auto robot = vsdk::RobotClient::at_address( + opt_robot_host.get(), {0, {std::move(dial_options)}}, inst.registry()); // Obtain a handle to the MLModelService module on the robot. Note that the string // `yamnet_classification_tflite` is arbitrary. It just matches what was used to name the diff --git a/src/viam/examples/modules/complex/client.cpp b/src/viam/examples/modules/complex/client.cpp index ed321abd5..ea3444844 100644 --- a/src/viam/examples/modules/complex/client.cpp +++ b/src/viam/examples/modules/complex/client.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -21,6 +22,10 @@ using namespace viam::sdk; int main() { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + Instance inst; + const char* uri = "http://localhost:8080/"; // replace with your URI if connecting securely DialOptions dial_options; dial_options.set_allow_insecure_downgrade(true); // set to false if connecting securely @@ -37,11 +42,11 @@ int main() { // Register custom gizmo and summation clients so robot client can access resources // of that type from the server. - Registry::register_resource_client(); - Registry::register_resource_client(); + inst.registry()->register_resource_client(); + inst.registry()->register_resource_client(); // Connect to robot. - std::shared_ptr robot = RobotClient::at_address(address, options); + std::shared_ptr robot = RobotClient::at_address(address, options, inst.registry()); // Print resources. std::cout << "Resources" << std::endl; std::vector resource_names = robot->resource_names(); diff --git a/src/viam/examples/modules/complex/main.cpp b/src/viam/examples/modules/complex/main.cpp index 59f590733..5d3f85f67 100644 --- a/src/viam/examples/modules/complex/main.cpp +++ b/src/viam/examples/modules/complex/main.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -24,11 +25,15 @@ using namespace viam::sdk; int main(int argc, char** argv) { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + Instance inst; + Model mybase_model("viam", "base", "mybase"); // Make sure to explicity register resources with custom APIs. - Registry::register_resource_server(); - Registry::register_resource_server(); + inst.registry()->register_resource_server(); + inst.registry()->register_resource_server(); std::shared_ptr mybase_mr = std::make_shared( API::get(), @@ -51,7 +56,7 @@ int main(int argc, char** argv) { }); std::vector> mrs = {mybase_mr, mygizmo_mr, mysummation_mr}; - auto my_mod = std::make_shared(argc, argv, mrs); + auto my_mod = std::make_shared(argc, argv, mrs, inst.registry()); my_mod->serve(); return EXIT_SUCCESS; diff --git a/src/viam/examples/modules/complex/test_complex_module.cpp b/src/viam/examples/modules/complex/test_complex_module.cpp index 4984df8f0..228ef96c7 100644 --- a/src/viam/examples/modules/complex/test_complex_module.cpp +++ b/src/viam/examples/modules/complex/test_complex_module.cpp @@ -24,8 +24,9 @@ using namespace viam::sdktests; struct RegisterGizmoAndSummationFixture { RegisterGizmoAndSummationFixture() { - Registry::register_resource(); - Registry::register_resource(); + auto* registry = GlobalInstance::registry(); + registry->register_resource(); + registry->register_resource(); } // Test teardown is a noop; diff --git a/src/viam/examples/motor/example_motor.cpp b/src/viam/examples/motor/example_motor.cpp index c4d8023d5..1a5ff50b7 100644 --- a/src/viam/examples/motor/example_motor.cpp +++ b/src/viam/examples/motor/example_motor.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -27,6 +28,11 @@ int main() { namespace vs = ::viam::sdk; try { + // Every Viam C++ SDK program must have one and only one Instance object which is created + // before any other C++ SDK objects and stays alive until all Viam C++ SDK objects are + // destroyed. + vs::Instance inst; + // 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"); @@ -49,7 +55,7 @@ int main() { std::shared_ptr robot; try { - robot = vs::RobotClient::at_address(robot_address, options); + robot = vs::RobotClient::at_address(robot_address, options, inst.registry()); cout << "Successfully connected to the robot" << endl; } catch (const std::exception& e) { cerr << "Failed to connect to the robot. Exiting." << endl; diff --git a/src/viam/sdk/common/instance.cpp b/src/viam/sdk/common/instance.cpp index 3c277273d..c356aee54 100644 --- a/src/viam/sdk/common/instance.cpp +++ b/src/viam/sdk/common/instance.cpp @@ -1,14 +1,22 @@ #include +#include + namespace viam { namespace sdk { -Instance::Instance() { - registry_.initialize(); +struct Instance::Impl { + Registry registry; +}; + +Instance::Instance() : impl_(std::make_unique()) { + impl_->registry.initialize(); } -Registry& Instance::registry() { - return registry_; +Instance::~Instance() = default; + +Registry* Instance::registry() { + return &(impl_->registry); } } // namespace sdk diff --git a/src/viam/sdk/common/instance.hpp b/src/viam/sdk/common/instance.hpp index 8297b74e4..0702d513f 100644 --- a/src/viam/sdk/common/instance.hpp +++ b/src/viam/sdk/common/instance.hpp @@ -1,10 +1,12 @@ #pragma once -#include +#include namespace viam { namespace sdk { +class Registry; + /// @brief Instance management for Viam C++ SDK applications. /// This is a single instance class which is responsible for global setup and teardown related to /// the SDK. An Instance must be constructed before doing anything else in a program, and it must @@ -13,11 +15,13 @@ namespace sdk { class Instance { public: Instance(); + ~Instance(); - Registry& registry(); + Registry* registry(); private: - Registry registry_; + struct Impl; + std::unique_ptr impl_; }; } // namespace sdk diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 526beecb5..041fb90b3 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -57,7 +57,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { std::shared_ptr res; const Dependencies deps = parent.get_dependencies_(&request->dependencies(), cfg.name()); const std::shared_ptr reg = - parent.registry_.lookup_model(cfg.api(), cfg.model()); + parent.registry_->lookup_model(cfg.api(), cfg.model()); if (reg) { try { res = reg->construct_resource(deps, cfg); @@ -112,7 +112,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { } const std::shared_ptr reg = - parent.registry_.lookup_model(cfg.name()); + parent.registry_->lookup_model(cfg.name()); if (reg) { try { const std::shared_ptr res = reg->construct_resource(deps, cfg); @@ -132,7 +132,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { ResourceConfig cfg = from_proto(proto); const std::shared_ptr reg = - parent.registry_.lookup_model(cfg.api(), cfg.model()); + parent.registry_->lookup_model(cfg.api(), cfg.model()); if (!reg) { return grpc::Status(grpc::UNKNOWN, "unable to validate resource " + cfg.resource_name().name() + @@ -214,17 +214,17 @@ std::shared_ptr ModuleService::get_parent_resource_(const Name& name) return parent_->resource_by_name(name); } -ModuleService::ModuleService(std::string addr, Registry& registry) +ModuleService::ModuleService(std::string addr, Registry* registry) : registry_(registry), module_(std::make_unique(std::move(addr))), - server_(std::make_unique(®istry_)) { + server_(std::make_unique(registry_)) { impl_ = std::make_unique(*this); } ModuleService::ModuleService(int argc, char** argv, const std::vector>& registrations, - Registry& registry) + Registry* registry) : ModuleService( [argc, argv] { if (argc < 2) { @@ -237,7 +237,7 @@ ModuleService::ModuleService(int argc, set_logger_severity_from_args(argc, argv); for (auto&& mr : registrations) { - registry_.register_model(mr); + registry_->register_model(mr); add_model_from_registry(mr->api(), mr->model()); } } @@ -280,7 +280,7 @@ void ModuleService::add_model_from_registry_inlock_(API api, Model model, const std::lock_guard&) { const std::shared_ptr creator = - registry_.lookup_resource_server(api); + registry_->lookup_resource_server(api); std::string name; if (creator && creator->service_descriptor()) { name = creator->service_descriptor()->full_name(); diff --git a/src/viam/sdk/module/service.hpp b/src/viam/sdk/module/service.hpp index cffc47fe7..0927d26a8 100644 --- a/src/viam/sdk/module/service.hpp +++ b/src/viam/sdk/module/service.hpp @@ -31,7 +31,7 @@ class ModuleService { public: /// @brief Creates a new ModuleService that can serve on the provided socket. /// @param addr Address of socket to serve on. - explicit ModuleService(std::string addr, Registry& registry); + explicit ModuleService(std::string addr, Registry* registry); /// @brief Creates a new ModuleService. Socket path and log level will be /// inferred from passed in command line arguments, and passed in model @@ -42,7 +42,7 @@ class ModuleService { explicit ModuleService(int argc, char** argv, const std::vector>& registrations, - Registry& registry); + Registry* registry); ~ModuleService(); /// @brief Starts module. serve will return when SIGINT or SIGTERM is received @@ -65,7 +65,7 @@ class ModuleService { std::string const& resource_name); std::shared_ptr get_parent_resource_(const Name& name); - Registry& registry_; + Registry* registry_; std::mutex lock_; std::unique_ptr module_; diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index 75e2324e3..43fa5c38f 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -185,7 +185,6 @@ class Registry { private: friend class Instance; - Registry() = default; /// @brief Initialized the Viam registry. No-op if it has already been called. diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index 6ac049eba..cb72451c7 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -178,7 +178,7 @@ void RobotClient::refresh() { // are being properly registered from name.subtype(), or update what we're // using for lookup const std::shared_ptr rs = - registry_.lookup_resource_client({name.namespace_(), name.type(), name.subtype()}); + registry_->lookup_resource_client({name.namespace_(), name.type(), name.subtype()}); if (rs) { try { const std::shared_ptr rpc_client = @@ -221,7 +221,7 @@ void RobotClient::refresh_every() { } }; -RobotClient::RobotClient(std::shared_ptr channel, Registry& registry) +RobotClient::RobotClient(std::shared_ptr channel, Registry* registry) : registry_(registry), channel_(channel->channel()), viam_channel_(std::move(channel)), @@ -235,7 +235,7 @@ std::vector RobotClient::resource_names() const { std::shared_ptr RobotClient::with_channel(std::shared_ptr channel, const Options& options, - Registry& registry) { + Registry* registry) { std::shared_ptr robot = std::make_shared(std::move(channel), registry); robot->refresh_interval_ = options.refresh_interval(); @@ -256,7 +256,7 @@ std::shared_ptr RobotClient::with_channel(std::shared_ptr RobotClient::at_address(const std::string& address, const Options& options, - Registry& registry) { + Registry* registry) { const char* uri = address.c_str(); auto channel = ViamChannel::dial(uri, options.dial_options()); std::shared_ptr robot = RobotClient::with_channel(channel, options, registry); @@ -267,7 +267,7 @@ std::shared_ptr RobotClient::at_address(const std::string& address, std::shared_ptr RobotClient::at_local_socket(const std::string& address, const Options& options, - Registry& registry) { + Registry* registry) { const std::string addr = "unix://" + address; const char* uri = addr.c_str(); const std::shared_ptr channel = diff --git a/src/viam/sdk/robot/client.hpp b/src/viam/sdk/robot/client.hpp index 09834e1d8..0a9610ac2 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -12,14 +12,16 @@ #include #include #include -#include #include +#include #include #include namespace viam { namespace sdk { +class Registry; + /// @defgroup Robot Classes related to a Robot representation. /// @class RobotClient client.hpp "robot/client.hpp" @@ -69,7 +71,7 @@ class RobotClient { /// @param options Options for connecting and refreshing. static std::shared_ptr at_address(const std::string& address, const Options& options, - Registry& registry); + Registry* registry); /// @brief Creates a robot client connected to the robot at the provided local socket. /// @param address The local socket of the robot (a .sock file, etc.). @@ -78,7 +80,7 @@ class RobotClient { /// Only useful for connecting to robots across Unix sockets. static std::shared_ptr at_local_socket(const std::string& address, const Options& options, - Registry& registry); + Registry* registry); /// @brief Creates a robot client connected to the provided channel. /// @param channel The channel to connect with. @@ -87,9 +89,9 @@ class RobotClient { /// `close()`d manually. static std::shared_ptr with_channel(std::shared_ptr channel, const Options& options, - Registry& registry); + Registry* registry); - RobotClient(std::shared_ptr channel, Registry& registry); + RobotClient(std::shared_ptr channel, Registry* registry); std::vector resource_names() const; @@ -152,7 +154,7 @@ class RobotClient { status get_machine_status() const; private: - Registry& registry_; + Registry* registry_; std::vector> threads_; diff --git a/src/viam/sdk/tests/mocks/mock_robot.cpp b/src/viam/sdk/tests/mocks/mock_robot.cpp index 8e28b944b..fdc068ec3 100644 --- a/src/viam/sdk/tests/mocks/mock_robot.cpp +++ b/src/viam/sdk/tests/mocks/mock_robot.cpp @@ -26,7 +26,7 @@ std::vector registered_models_for_resource(const std::shared_ptr std::string resource_type; std::string resource_subtype; std::vector resource_names; - for (const auto& kv : GlobalInstance::registry().registered_models()) { + for (const auto& kv : GlobalInstance::registry()->registered_models()) { const std::shared_ptr reg = kv.second; if (reg->api() == resource->api()) { resource_type = reg->api().resource_type(); diff --git a/src/viam/sdk/tests/test_robot.cpp b/src/viam/sdk/tests/test_robot.cpp index e7ccc0de1..a7b403359 100644 --- a/src/viam/sdk/tests/test_robot.cpp +++ b/src/viam/sdk/tests/test_robot.cpp @@ -42,7 +42,7 @@ void robot_client_to_mocks_pipeline(F&& test_case) { rm->add(std::string("mock_generic"), generic::MockGenericComponent::get_mock_generic()); 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(&GlobalInstance::registry()); + auto server = std::make_shared(GlobalInstance::registry()); MockRobotService service(rm, *server); server->start(); @@ -63,7 +63,7 @@ void robot_client_to_mocks_pipeline(F&& test_case) { } BOOST_AUTO_TEST_CASE(test_registering_resources) { - auto& registry = GlobalInstance::registry(); + auto& registry = *GlobalInstance::registry(); // To test with mock resources we need to be able to create them, which means registering // constructors. This tests that we register correctly. diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index c24bed482..655b34904 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -17,7 +17,7 @@ struct GlobalInstance { return inst; } - static sdk::Registry& registry() { + static sdk::Registry* registry() { return get().registry(); } }; @@ -61,7 +61,7 @@ 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) { - auto server = std::make_shared(&GlobalInstance::registry()); + auto server = std::make_shared(GlobalInstance::registry()); // normally the high level server service (either robot or module) handles adding managed // resources, but in this case we must do it ourselves. @@ -74,7 +74,7 @@ void client_to_mock_pipeline(std::shared_ptr mock, F&& test_case) { auto grpc_channel = test_server.grpc_in_process_channel(); auto resource_client = GlobalInstance::registry() - .lookup_resource_client(API::get()) + ->lookup_resource_client(API::get()) ->create_rpc_client(mock->name(), std::move(grpc_channel)); // Run the passed-in test case on the created stack and give access to the From e5cf6d054c82a4dff634ec64b5eb95e3c2a5426c Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 25 Feb 2025 15:37:51 -0500 Subject: [PATCH 08/31] add instance management infra --- .../modules/complex/test_complex_module.cpp | 2 +- src/viam/sdk/common/instance.cpp | 41 ++++++++++++++++++- src/viam/sdk/common/instance.hpp | 2 + src/viam/sdk/tests/mocks/mock_robot.cpp | 2 +- src/viam/sdk/tests/test_robot.cpp | 6 +-- src/viam/sdk/tests/test_utils.hpp | 16 ++------ 6 files changed, 49 insertions(+), 20 deletions(-) diff --git a/src/viam/examples/modules/complex/test_complex_module.cpp b/src/viam/examples/modules/complex/test_complex_module.cpp index 228ef96c7..3f4d4ee3d 100644 --- a/src/viam/examples/modules/complex/test_complex_module.cpp +++ b/src/viam/examples/modules/complex/test_complex_module.cpp @@ -24,7 +24,7 @@ using namespace viam::sdktests; struct RegisterGizmoAndSummationFixture { RegisterGizmoAndSummationFixture() { - auto* registry = GlobalInstance::registry(); + auto* registry = Instance::current().registry(); registry->register_resource(); registry->register_resource(); } diff --git a/src/viam/sdk/common/instance.cpp b/src/viam/sdk/common/instance.cpp index c356aee54..9b6a7eccc 100644 --- a/src/viam/sdk/common/instance.cpp +++ b/src/viam/sdk/common/instance.cpp @@ -1,19 +1,56 @@ #include +#include #include +#include + namespace viam { namespace sdk { +namespace { + +// Memory region sentinel to check if object is being destroyed. +std::aligned_storage_t sentinel; + +std::atomic current_instance{nullptr}; + +} // namespace + struct Instance::Impl { Registry registry; }; -Instance::Instance() : impl_(std::make_unique()) { +Instance::Instance() { + Instance* expected = nullptr; + + if (!current_instance.compare_exchange_strong(expected, this)) { + throw Exception("tried to create duplicate instance"); + } + + impl_ = std::make_unique(); impl_->registry.initialize(); } -Instance::~Instance() = default; +Instance::~Instance() { + current_instance.store(reinterpret_cast(&sentinel)); + impl_.reset(); +} + +Instance& Instance::current() { + if (!current_instance.load()) { + // This variable declaration calls the default ctor, storing a current instance. + static Instance inst; + } + + Instance* current = current_instance.load(); + + if (current == reinterpret_cast(&sentinel)) { + throw Exception("instance was destroyed"); + } + + return *current; +} Registry* Instance::registry() { return &(impl_->registry); diff --git a/src/viam/sdk/common/instance.hpp b/src/viam/sdk/common/instance.hpp index 0702d513f..c1ab0961f 100644 --- a/src/viam/sdk/common/instance.hpp +++ b/src/viam/sdk/common/instance.hpp @@ -17,6 +17,8 @@ class Instance { Instance(); ~Instance(); + static Instance& current(); + Registry* registry(); private: diff --git a/src/viam/sdk/tests/mocks/mock_robot.cpp b/src/viam/sdk/tests/mocks/mock_robot.cpp index fdc068ec3..5dc03fcc2 100644 --- a/src/viam/sdk/tests/mocks/mock_robot.cpp +++ b/src/viam/sdk/tests/mocks/mock_robot.cpp @@ -26,7 +26,7 @@ std::vector registered_models_for_resource(const std::shared_ptr std::string resource_type; std::string resource_subtype; std::vector resource_names; - for (const auto& kv : GlobalInstance::registry()->registered_models()) { + for (const auto& kv : Instance::current().registry()->registered_models()) { const std::shared_ptr reg = kv.second; if (reg->api() == resource->api()) { resource_type = reg->api().resource_type(); diff --git a/src/viam/sdk/tests/test_robot.cpp b/src/viam/sdk/tests/test_robot.cpp index a7b403359..e3bead0c9 100644 --- a/src/viam/sdk/tests/test_robot.cpp +++ b/src/viam/sdk/tests/test_robot.cpp @@ -42,7 +42,7 @@ void robot_client_to_mocks_pipeline(F&& test_case) { rm->add(std::string("mock_generic"), generic::MockGenericComponent::get_mock_generic()); 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(GlobalInstance::registry()); + auto server = std::make_shared(Instance::current().registry()); MockRobotService service(rm, *server); server->start(); @@ -52,7 +52,7 @@ void robot_client_to_mocks_pipeline(F&& test_case) { auto grpc_channel = test_server.grpc_in_process_channel(); auto viam_channel = std::make_shared(grpc_channel, "", nullptr); auto client = RobotClient::with_channel( - viam_channel, Options(0, boost::none), GlobalInstance::registry()); + viam_channel, Options(0, boost::none), Instance::current().registry()); // Run the passed-in test case on the created stack and give access to the // created RobotClient and MockRobotService. @@ -63,7 +63,7 @@ void robot_client_to_mocks_pipeline(F&& test_case) { } BOOST_AUTO_TEST_CASE(test_registering_resources) { - auto& registry = *GlobalInstance::registry(); + auto& registry = *Instance::current().registry(); // To test with mock resources we need to be able to create them, which means registering // constructors. This tests that we register correctly. diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index 655b34904..e5c6fb729 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -11,17 +11,6 @@ namespace viam { namespace sdktests { -struct GlobalInstance { - static sdk::Instance& get() { - static sdk::Instance inst; - return inst; - } - - static sdk::Registry* registry() { - return get().registry(); - } -}; - using namespace viam::sdk; ProtoStruct fake_map(); @@ -61,7 +50,7 @@ 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) { - auto server = std::make_shared(GlobalInstance::registry()); + auto server = std::make_shared(sdk::Instance::current().registry()); // normally the high level server service (either robot or module) handles adding managed // resources, but in this case we must do it ourselves. @@ -73,7 +62,8 @@ void client_to_mock_pipeline(std::shared_ptr mock, F&& test_case) { auto test_server = TestServer(server); auto grpc_channel = test_server.grpc_in_process_channel(); - auto resource_client = GlobalInstance::registry() + auto resource_client = sdk::Instance::current() + .registry() ->lookup_resource_client(API::get()) ->create_rpc_client(mock->name(), std::move(grpc_channel)); From 5024d3a2d6be053ee8b088ede5746475c2298477 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 25 Feb 2025 15:43:04 -0500 Subject: [PATCH 09/31] update tflite module --- src/viam/examples/modules/tflite/main.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/viam/examples/modules/tflite/main.cpp b/src/viam/examples/modules/tflite/main.cpp index 44cd8e284..e1c806354 100644 --- a/src/viam/examples/modules/tflite/main.cpp +++ b/src/viam/examples/modules/tflite/main.cpp @@ -23,6 +23,7 @@ #include +#include #include #include #include @@ -723,6 +724,10 @@ class MLModelServiceTFLite : public vsdk::MLModelService, }; int serve(const std::string& socket_path) try { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + vsdk::Instance inst; + // Create a new model registration for the service. auto module_registration = std::make_shared( // Identify that this resource offers the MLModelService API @@ -737,10 +742,10 @@ int serve(const std::string& socket_path) try { }); // Register the newly created registration with the Registry. - vsdk::Registry::register_model(module_registration); + inst.registry()->register_model(module_registration); // Construct the module service and tell it where to place the socket path. - auto module_service = std::make_shared(socket_path); + auto module_service = std::make_shared(socket_path, inst.registry()); // Add the server as providing the API and model declared in the // registration. From ef8ec8eff50e91c87c23dcc3a34f26611fe48ccc Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 25 Feb 2025 15:59:35 -0500 Subject: [PATCH 10/31] silence spurious const warning --- src/viam/sdk/common/instance.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/sdk/common/instance.cpp b/src/viam/sdk/common/instance.cpp index 9b6a7eccc..c366ad6c6 100644 --- a/src/viam/sdk/common/instance.cpp +++ b/src/viam/sdk/common/instance.cpp @@ -40,7 +40,7 @@ Instance::~Instance() { Instance& Instance::current() { if (!current_instance.load()) { // This variable declaration calls the default ctor, storing a current instance. - static Instance inst; + static Instance inst; // NOLINT (misc-const-correctness) } Instance* current = current_instance.load(); From 16113fd4dd38f44e8481c3468eba258b22296941 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Tue, 25 Feb 2025 16:27:29 -0500 Subject: [PATCH 11/31] make method static --- src/viam/sdk/registry/registry.cpp | 2 +- src/viam/sdk/registry/registry.hpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 71e3e7711..db28f628b 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -163,7 +163,7 @@ std::shared_ptr Registry::lookup_resource_clie } const google::protobuf::ServiceDescriptor* Registry::get_service_descriptor_( - const char* service_full_name) const { + 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) { diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index 43fa5c38f..db476be97 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -204,8 +204,8 @@ class Registry { void register_resource_client_( API api, std::shared_ptr resource_registration); - const google::protobuf::ServiceDescriptor* get_service_descriptor_( - const char* service_full_name) const; + static const google::protobuf::ServiceDescriptor* get_service_descriptor_( + const char* service_full_name); std::shared_ptr lookup_model_inlock_( const std::string& name, const std::lock_guard&) const; From 4771390555ed6fdad2220d138e4ab770bb8b7b43 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Thu, 27 Feb 2025 13:48:18 -0500 Subject: [PATCH 12/31] use static current to populate registry member --- src/viam/examples/camera/example_camera.cpp | 8 +----- src/viam/examples/dial/example_dial.cpp | 7 +---- .../dial_api_key/example_dial_api_key.cpp | 7 +---- .../example_audio_classification_client.cpp | 9 ++----- src/viam/examples/modules/complex/client.cpp | 11 +++----- src/viam/examples/modules/complex/main.cpp | 11 +++----- .../modules/complex/test_complex_module.cpp | 5 ++-- src/viam/examples/modules/simple/client.cpp | 5 +--- src/viam/examples/modules/simple/main.cpp | 5 +--- src/viam/examples/modules/tflite/main.cpp | 9 ++----- src/viam/examples/motor/example_motor.cpp | 8 +----- src/viam/sdk/module/service.cpp | 27 +++++++++---------- src/viam/sdk/module/service.hpp | 5 ++-- src/viam/sdk/registry/registry.cpp | 5 ++++ src/viam/sdk/registry/registry.hpp | 3 +++ src/viam/sdk/robot/client.cpp | 20 ++++++-------- src/viam/sdk/robot/client.hpp | 24 ++++------------- src/viam/sdk/rpc/server.cpp | 4 +-- src/viam/sdk/rpc/server.hpp | 4 +-- src/viam/sdk/tests/test_robot.cpp | 7 +++-- src/viam/sdk/tests/test_utils.hpp | 7 +++-- 21 files changed, 62 insertions(+), 129 deletions(-) diff --git a/src/viam/examples/camera/example_camera.cpp b/src/viam/examples/camera/example_camera.cpp index b0202a27e..bc1b0ab9a 100644 --- a/src/viam/examples/camera/example_camera.cpp +++ b/src/viam/examples/camera/example_camera.cpp @@ -3,7 +3,6 @@ #include #include -#include #include #include #include @@ -14,11 +13,6 @@ int main() { using std::endl; namespace vs = ::viam::sdk; try { - // Every Viam C++ SDK program must have one and only one Instance object which is created - // before - // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. - vs::Instance inst; - // 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"); @@ -41,7 +35,7 @@ int main() { std::shared_ptr robot; try { - robot = vs::RobotClient::at_address(robot_address, options, inst.registry()); + robot = vs::RobotClient::at_address(robot_address, options); cout << "Successfully connected to the robot" << endl; } catch (const std::exception& e) { cerr << "Failed to connect to the robot. Exiting." << endl; diff --git a/src/viam/examples/dial/example_dial.cpp b/src/viam/examples/dial/example_dial.cpp index 28ccd7659..efa783dfb 100644 --- a/src/viam/examples/dial/example_dial.cpp +++ b/src/viam/examples/dial/example_dial.cpp @@ -9,7 +9,6 @@ #include -#include #include #include #include @@ -17,10 +16,6 @@ using namespace viam::sdk; int main() { - // Every Viam C++ SDK program must have one and only one Instance object which is created before - // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. - Instance inst; - const char* uri = ""; DialOptions dial_options; std::string type = ""; @@ -32,7 +27,7 @@ int main() { Options options(1, opts); // connect to robot, ensure we can refresh it - std::shared_ptr robot = RobotClient::at_address(address, options, inst.registry()); + std::shared_ptr robot = RobotClient::at_address(address, options); // ensure we can query resources std::vector resource_names = robot->resource_names(); 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 7c9e82fef..4c47beffe 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 @@ -10,7 +10,6 @@ #include #include -#include #include #include @@ -19,10 +18,6 @@ using namespace viam::sdk; namespace po = boost::program_options; int main(int argc, char* argv[]) { - // Every Viam C++ SDK program must have one and only one Instance object which is created before - // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. - Instance inst; - po::options_description desc("Allowed options"); desc.add_options()("help", "List options and exit")( "uri", po::value(), "URI of robot")( @@ -46,7 +41,7 @@ int main(int argc, char* argv[]) { // connect to robot, ensure we can refresh it std::shared_ptr robot = - RobotClient::at_address(vm["uri"].as(), options, inst.registry()); + RobotClient::at_address(vm["uri"].as(), options); // ensure we can query resources std::vector resource_names = robot->resource_names(); diff --git a/src/viam/examples/mlmodel/example_audio_classification_client.cpp b/src/viam/examples/mlmodel/example_audio_classification_client.cpp index ab06a082e..95e26f57c 100644 --- a/src/viam/examples/mlmodel/example_audio_classification_client.cpp +++ b/src/viam/examples/mlmodel/example_audio_classification_client.cpp @@ -29,7 +29,6 @@ #include #include -#include #include #include @@ -79,10 +78,6 @@ constexpr char kRobotConfigTemplate[] = R"( } // namespace int main(int argc, char* argv[]) try { - // Every Viam C++ SDK program must have one and only one Instance object which is created before - // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. - viam::sdk::Instance inst; - // Build up our command line options. The example operates in two // modes. In the "--generate" mode, it takes command line // parameters needed to satisfy the interpolation points in the @@ -239,8 +234,8 @@ int main(int argc, char* argv[]) try { dial_options.set_entity(opt_api_key_id.get()); dial_options.set_credentials(viam::sdk::Credentials("api-key", opt_api_key.get())); - auto robot = vsdk::RobotClient::at_address( - opt_robot_host.get(), {0, {std::move(dial_options)}}, inst.registry()); + auto robot = + vsdk::RobotClient::at_address(opt_robot_host.get(), {0, {std::move(dial_options)}}); // Obtain a handle to the MLModelService module on the robot. Note that the string // `yamnet_classification_tflite` is arbitrary. It just matches what was used to name the diff --git a/src/viam/examples/modules/complex/client.cpp b/src/viam/examples/modules/complex/client.cpp index ea3444844..ac85c9f00 100644 --- a/src/viam/examples/modules/complex/client.cpp +++ b/src/viam/examples/modules/complex/client.cpp @@ -11,7 +11,6 @@ #include #include -#include #include #include #include @@ -22,10 +21,6 @@ using namespace viam::sdk; int main() { - // Every Viam C++ SDK program must have one and only one Instance object which is created before - // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. - Instance inst; - const char* uri = "http://localhost:8080/"; // replace with your URI if connecting securely DialOptions dial_options; dial_options.set_allow_insecure_downgrade(true); // set to false if connecting securely @@ -42,11 +37,11 @@ int main() { // Register custom gizmo and summation clients so robot client can access resources // of that type from the server. - inst.registry()->register_resource_client(); - inst.registry()->register_resource_client(); + Registry::get().register_resource_client(); + Registry::get().register_resource_client(); // Connect to robot. - std::shared_ptr robot = RobotClient::at_address(address, options, inst.registry()); + std::shared_ptr robot = RobotClient::at_address(address, options); // Print resources. std::cout << "Resources" << std::endl; std::vector resource_names = robot->resource_names(); diff --git a/src/viam/examples/modules/complex/main.cpp b/src/viam/examples/modules/complex/main.cpp index 5d3f85f67..da80453e6 100644 --- a/src/viam/examples/modules/complex/main.cpp +++ b/src/viam/examples/modules/complex/main.cpp @@ -5,7 +5,6 @@ #include #include -#include #include #include #include @@ -25,15 +24,11 @@ using namespace viam::sdk; int main(int argc, char** argv) { - // Every Viam C++ SDK program must have one and only one Instance object which is created before - // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. - Instance inst; - Model mybase_model("viam", "base", "mybase"); // Make sure to explicity register resources with custom APIs. - inst.registry()->register_resource_server(); - inst.registry()->register_resource_server(); + Registry::get().register_resource_server(); + Registry::get().register_resource_server(); std::shared_ptr mybase_mr = std::make_shared( API::get(), @@ -56,7 +51,7 @@ int main(int argc, char** argv) { }); std::vector> mrs = {mybase_mr, mygizmo_mr, mysummation_mr}; - auto my_mod = std::make_shared(argc, argv, mrs, inst.registry()); + auto my_mod = std::make_shared(argc, argv, mrs); my_mod->serve(); return EXIT_SUCCESS; diff --git a/src/viam/examples/modules/complex/test_complex_module.cpp b/src/viam/examples/modules/complex/test_complex_module.cpp index 3f4d4ee3d..14d68a8cf 100644 --- a/src/viam/examples/modules/complex/test_complex_module.cpp +++ b/src/viam/examples/modules/complex/test_complex_module.cpp @@ -24,9 +24,8 @@ using namespace viam::sdktests; struct RegisterGizmoAndSummationFixture { RegisterGizmoAndSummationFixture() { - auto* registry = Instance::current().registry(); - registry->register_resource(); - registry->register_resource(); + Registry::get().register_resource(); + Registry::get().register_resource(); } // Test teardown is a noop; diff --git a/src/viam/examples/modules/simple/client.cpp b/src/viam/examples/modules/simple/client.cpp index ac014d1e7..fcaab5eff 100644 --- a/src/viam/examples/modules/simple/client.cpp +++ b/src/viam/examples/modules/simple/client.cpp @@ -2,7 +2,6 @@ #include #include -#include #include #include #include @@ -11,8 +10,6 @@ using namespace viam::sdk; int main() { - Instance inst; - const char* uri = "http://localhost:8080/"; // replace with your URI if connecting securely DialOptions dial_options; dial_options.set_allow_insecure_downgrade(true); // set to false if connecting securely @@ -27,7 +24,7 @@ int main() { std::string address(uri); Options options(1, opts); - std::shared_ptr robot = RobotClient::at_address(address, options, inst.registry()); + std::shared_ptr robot = RobotClient::at_address(address, options); // Print resources std::cout << "Resources\n"; diff --git a/src/viam/examples/modules/simple/main.cpp b/src/viam/examples/modules/simple/main.cpp index b899d3b44..cfd783f66 100644 --- a/src/viam/examples/modules/simple/main.cpp +++ b/src/viam/examples/modules/simple/main.cpp @@ -3,7 +3,6 @@ #include #include -#include #include #include #include @@ -76,8 +75,6 @@ ProtoStruct MySensor::get_readings(const ProtoStruct&) { } int main(int argc, char** argv) try { - Instance inst; - Model mysensor_model("viam", "sensor", "mysensor"); std::shared_ptr mr = std::make_shared( @@ -87,7 +84,7 @@ int main(int argc, char** argv) try { &MySensor::validate); std::vector> mrs = {mr}; - auto my_mod = std::make_shared(argc, argv, mrs, inst.registry()); + auto my_mod = std::make_shared(argc, argv, mrs); my_mod->serve(); return EXIT_SUCCESS; diff --git a/src/viam/examples/modules/tflite/main.cpp b/src/viam/examples/modules/tflite/main.cpp index e1c806354..44cd8e284 100644 --- a/src/viam/examples/modules/tflite/main.cpp +++ b/src/viam/examples/modules/tflite/main.cpp @@ -23,7 +23,6 @@ #include -#include #include #include #include @@ -724,10 +723,6 @@ class MLModelServiceTFLite : public vsdk::MLModelService, }; int serve(const std::string& socket_path) try { - // Every Viam C++ SDK program must have one and only one Instance object which is created before - // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. - vsdk::Instance inst; - // Create a new model registration for the service. auto module_registration = std::make_shared( // Identify that this resource offers the MLModelService API @@ -742,10 +737,10 @@ int serve(const std::string& socket_path) try { }); // Register the newly created registration with the Registry. - inst.registry()->register_model(module_registration); + vsdk::Registry::register_model(module_registration); // Construct the module service and tell it where to place the socket path. - auto module_service = std::make_shared(socket_path, inst.registry()); + auto module_service = std::make_shared(socket_path); // Add the server as providing the API and model declared in the // registration. diff --git a/src/viam/examples/motor/example_motor.cpp b/src/viam/examples/motor/example_motor.cpp index 1a5ff50b7..c4d8023d5 100644 --- a/src/viam/examples/motor/example_motor.cpp +++ b/src/viam/examples/motor/example_motor.cpp @@ -6,7 +6,6 @@ #include #include -#include #include #include #include @@ -28,11 +27,6 @@ int main() { namespace vs = ::viam::sdk; try { - // Every Viam C++ SDK program must have one and only one Instance object which is created - // before any other C++ SDK objects and stays alive until all Viam C++ SDK objects are - // destroyed. - vs::Instance inst; - // 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"); @@ -55,7 +49,7 @@ int main() { std::shared_ptr robot; try { - robot = vs::RobotClient::at_address(robot_address, options, inst.registry()); + robot = vs::RobotClient::at_address(robot_address, options); cout << "Successfully connected to the robot" << endl; } catch (const std::exception& e) { cerr << "Failed to connect to the robot. Exiting." << endl; diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 041fb90b3..d644b5420 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -208,32 +208,29 @@ Dependencies ModuleService::get_dependencies_( std::shared_ptr ModuleService::get_parent_resource_(const Name& name) { if (!parent_) { - parent_ = RobotClient::at_local_socket(parent_addr_, {0, boost::none}, registry_); + parent_ = RobotClient::at_local_socket(parent_addr_, {0, boost::none}); } return parent_->resource_by_name(name); } -ModuleService::ModuleService(std::string addr, Registry* registry) - : registry_(registry), +ModuleService::ModuleService(std::string addr) + : registry_(&Registry::get()), module_(std::make_unique(std::move(addr))), - server_(std::make_unique(registry_)) { + server_(std::make_unique()) { impl_ = std::make_unique(*this); } ModuleService::ModuleService(int argc, char** argv, - const std::vector>& registrations, - Registry* registry) - : ModuleService( - [argc, argv] { - if (argc < 2) { - throw Exception(ErrorCondition::k_connection, - "Need socket path as command line argument"); - } - return argv[1]; - }(), - registry) { + const std::vector>& registrations) + : ModuleService([argc, argv] { + if (argc < 2) { + throw Exception(ErrorCondition::k_connection, + "Need socket path as command line argument"); + } + return argv[1]; + }()) { set_logger_severity_from_args(argc, argv); for (auto&& mr : registrations) { diff --git a/src/viam/sdk/module/service.hpp b/src/viam/sdk/module/service.hpp index 0927d26a8..95a7ad4f3 100644 --- a/src/viam/sdk/module/service.hpp +++ b/src/viam/sdk/module/service.hpp @@ -31,7 +31,7 @@ class ModuleService { public: /// @brief Creates a new ModuleService that can serve on the provided socket. /// @param addr Address of socket to serve on. - explicit ModuleService(std::string addr, Registry* registry); + explicit ModuleService(std::string addr); /// @brief Creates a new ModuleService. Socket path and log level will be /// inferred from passed in command line arguments, and passed in model @@ -41,8 +41,7 @@ class ModuleService { /// @param registrations Models to register and add to the module. explicit ModuleService(int argc, char** argv, - const std::vector>& registrations, - Registry* registry); + const std::vector>& registrations); ~ModuleService(); /// @brief Starts module. serve will return when SIGINT or SIGTERM is received diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 8f7209adf..1ee9b5533 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -90,6 +91,10 @@ const Model& ModelRegistration::model() const { return model_; }; +Registry& Registry::get() { + return *Instance::current().registry(); +} + void Registry::register_model(std::shared_ptr resource) { std::string reg_key = resource->api().to_string() + "/" + resource->model().to_string(); if (resources_.find(reg_key) != resources_.end()) { diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index db476be97..f09ee2d62 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -103,6 +103,9 @@ class ModelRegistration { /// @brief A registry of known resources. class Registry { public: + /// @brief Get the application-wide instance of Registry. + static Registry& get(); + /// @brief Registers a resource with the Registry. /// @param resource An object containing resource registration information. /// @throws `Exception` if the resource has already been registered. diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index cb72451c7..a8cb16c75 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -221,8 +221,8 @@ void RobotClient::refresh_every() { } }; -RobotClient::RobotClient(std::shared_ptr channel, Registry* registry) - : registry_(registry), +RobotClient::RobotClient(std::shared_ptr channel) + : registry_(&Registry::get()), channel_(channel->channel()), viam_channel_(std::move(channel)), should_close_channel_(false), @@ -234,10 +234,8 @@ std::vector RobotClient::resource_names() const { } std::shared_ptr RobotClient::with_channel(std::shared_ptr channel, - const Options& options, - Registry* registry) { - std::shared_ptr robot = - std::make_shared(std::move(channel), registry); + const Options& options) { + std::shared_ptr robot = std::make_shared(std::move(channel)); robot->refresh_interval_ = options.refresh_interval(); robot->should_refresh_ = (robot->refresh_interval_ > 0); if (robot->should_refresh_) { @@ -255,25 +253,23 @@ std::shared_ptr RobotClient::with_channel(std::shared_ptr RobotClient::at_address(const std::string& address, - const Options& options, - Registry* registry) { + const Options& options) { const char* uri = address.c_str(); auto channel = ViamChannel::dial(uri, options.dial_options()); - std::shared_ptr robot = RobotClient::with_channel(channel, options, registry); + std::shared_ptr robot = RobotClient::with_channel(channel, options); robot->should_close_channel_ = true; return robot; }; std::shared_ptr RobotClient::at_local_socket(const std::string& address, - const Options& options, - Registry* registry) { + const Options& options) { const std::string addr = "unix://" + address; const char* uri = addr.c_str(); const std::shared_ptr channel = sdk::impl::create_viam_channel(uri, grpc::InsecureChannelCredentials()); auto viam_channel = std::make_shared(channel, address.c_str(), nullptr); - std::shared_ptr robot = RobotClient::with_channel(viam_channel, options, registry); + std::shared_ptr robot = RobotClient::with_channel(viam_channel, options); robot->should_close_channel_ = true; return robot; diff --git a/src/viam/sdk/robot/client.hpp b/src/viam/sdk/robot/client.hpp index 0a9610ac2..410abca28 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -12,16 +12,14 @@ #include #include #include +#include #include -#include #include #include namespace viam { namespace sdk { -class Registry; - /// @defgroup Robot Classes related to a Robot representation. /// @class RobotClient client.hpp "robot/client.hpp" @@ -70,8 +68,7 @@ class RobotClient { /// @param address The address of the robot (IP address, URI, URL, etc.) /// @param options Options for connecting and refreshing. static std::shared_ptr at_address(const std::string& address, - const Options& options, - Registry* registry); + const Options& options); /// @brief Creates a robot client connected to the robot at the provided local socket. /// @param address The local socket of the robot (a .sock file, etc.). @@ -79,8 +76,7 @@ class RobotClient { /// Creates a direct connection to the robot using the `unix://` scheme. /// Only useful for connecting to robots across Unix sockets. static std::shared_ptr at_local_socket(const std::string& address, - const Options& options, - Registry* registry); + const Options& options); /// @brief Creates a robot client connected to the provided channel. /// @param channel The channel to connect with. @@ -88,11 +84,8 @@ class RobotClient { /// Connects directly to a pre-existing channel. A robot created this way must be /// `close()`d manually. static std::shared_ptr with_channel(std::shared_ptr channel, - const Options& options, - Registry* registry); - - RobotClient(std::shared_ptr channel, Registry* registry); - + const Options& options); + RobotClient(std::shared_ptr channel); std::vector resource_names() const; /// @brief Lookup and return a `shared_ptr` to a resource. @@ -155,24 +148,17 @@ class RobotClient { private: Registry* registry_; - std::vector> threads_; - std::atomic should_refresh_; unsigned int refresh_interval_; - std::shared_ptr channel_; std::shared_ptr viam_channel_; bool should_close_channel_; - struct impl; std::unique_ptr impl_; - mutable std::mutex lock_; - std::vector resource_names_; ResourceManager resource_manager_; - void refresh_every(); }; diff --git a/src/viam/sdk/rpc/server.cpp b/src/viam/sdk/rpc/server.cpp index b2c46c695..9964abbac 100644 --- a/src/viam/sdk/rpc/server.cpp +++ b/src/viam/sdk/rpc/server.cpp @@ -15,11 +15,11 @@ namespace viam { namespace sdk { -Server::Server(const Registry* registry) : builder_(std::make_unique()) { +Server::Server() : builder_(std::make_unique()) { builder_->SetMaxReceiveMessageSize(kMaxMessageSize); builder_->SetMaxSendMessageSize(kMaxMessageSize); builder_->SetMaxMessageSize(kMaxMessageSize); - for (const auto& rr : registry->registered_resource_servers()) { + for (const auto& rr : Registry::get().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/rpc/server.hpp b/src/viam/sdk/rpc/server.hpp index 5f868b779..e735a6351 100644 --- a/src/viam/sdk/rpc/server.hpp +++ b/src/viam/sdk/rpc/server.hpp @@ -23,13 +23,11 @@ class TestServer; namespace sdk { -class Registry; - /// @class Server server.hpp "rpc/server.hpp" /// @brief Defines gRPC `Server` functionality. class Server { public: - Server(const Registry* registry); + Server(); ~Server(); /// @brief Starts the grpc server. Can only be called once. diff --git a/src/viam/sdk/tests/test_robot.cpp b/src/viam/sdk/tests/test_robot.cpp index e3bead0c9..2018ed36b 100644 --- a/src/viam/sdk/tests/test_robot.cpp +++ b/src/viam/sdk/tests/test_robot.cpp @@ -42,7 +42,7 @@ void robot_client_to_mocks_pipeline(F&& test_case) { rm->add(std::string("mock_generic"), generic::MockGenericComponent::get_mock_generic()); 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(Instance::current().registry()); + auto server = std::make_shared(); MockRobotService service(rm, *server); server->start(); @@ -51,8 +51,7 @@ void robot_client_to_mocks_pipeline(F&& test_case) { auto test_server = TestServer(server); auto grpc_channel = test_server.grpc_in_process_channel(); auto viam_channel = std::make_shared(grpc_channel, "", nullptr); - auto client = RobotClient::with_channel( - viam_channel, Options(0, boost::none), Instance::current().registry()); + auto client = RobotClient::with_channel(viam_channel, Options(0, boost::none)); // Run the passed-in test case on the created stack and give access to the // created RobotClient and MockRobotService. @@ -63,7 +62,7 @@ void robot_client_to_mocks_pipeline(F&& test_case) { } BOOST_AUTO_TEST_CASE(test_registering_resources) { - auto& registry = *Instance::current().registry(); + auto& registry = Registry::get(); // To test with mock resources we need to be able to create them, which means registering // constructors. This tests that we register correctly. diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index 3de43aed8..26a51444e 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -51,7 +51,7 @@ 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) { - auto server = std::make_shared(sdk::Instance::current().registry()); + 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. @@ -63,9 +63,8 @@ void client_to_mock_pipeline(std::shared_ptr mock, F&& test_case) { auto test_server = TestServer(server); auto grpc_channel = test_server.grpc_in_process_channel(); - auto resource_client = sdk::Instance::current() - .registry() - ->lookup_resource_client(API::get()) + auto resource_client = sdk::Registry::get() + .lookup_resource_client(API::get()) ->create_rpc_client(mock->name(), std::move(grpc_channel)); // Run the passed-in test case on the created stack and give access to the From 18d26c92c4a0b39dc785c9306a3fcd803c736f0b Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Thu, 27 Feb 2025 13:51:47 -0500 Subject: [PATCH 13/31] revert and adapt examples --- src/viam/examples/camera/example_camera.cpp | 6 ++++++ src/viam/examples/dial/example_dial.cpp | 5 +++++ src/viam/examples/dial_api_key/example_dial_api_key.cpp | 5 +++++ .../mlmodel/example_audio_classification_client.cpp | 5 +++++ src/viam/examples/modules/complex/client.cpp | 5 +++++ src/viam/examples/modules/complex/main.cpp | 9 +++++++-- .../examples/modules/complex/test_complex_module.cpp | 5 +++-- src/viam/examples/modules/simple/client.cpp | 3 +++ src/viam/examples/modules/simple/main.cpp | 3 +++ src/viam/examples/modules/tflite/main.cpp | 9 +++++++-- src/viam/examples/motor/example_motor.cpp | 6 ++++++ 11 files changed, 55 insertions(+), 6 deletions(-) diff --git a/src/viam/examples/camera/example_camera.cpp b/src/viam/examples/camera/example_camera.cpp index bc1b0ab9a..d1eb9b80c 100644 --- a/src/viam/examples/camera/example_camera.cpp +++ b/src/viam/examples/camera/example_camera.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -13,6 +14,11 @@ int main() { using std::endl; namespace vs = ::viam::sdk; try { + // Every Viam C++ SDK program must have one and only one Instance object which is created + // before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + vs::Instance inst; + // 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 efa783dfb..f39522c26 100644 --- a/src/viam/examples/dial/example_dial.cpp +++ b/src/viam/examples/dial/example_dial.cpp @@ -9,6 +9,7 @@ #include +#include #include #include #include @@ -16,6 +17,10 @@ using namespace viam::sdk; int main() { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + Instance inst; + const char* uri = ""; DialOptions dial_options; std::string type = ""; 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 4c47beffe..b723e4227 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 @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -18,6 +19,10 @@ using namespace viam::sdk; namespace po = boost::program_options; int main(int argc, char* argv[]) { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + Instance inst; + po::options_description desc("Allowed options"); desc.add_options()("help", "List options and exit")( "uri", po::value(), "URI of robot")( diff --git a/src/viam/examples/mlmodel/example_audio_classification_client.cpp b/src/viam/examples/mlmodel/example_audio_classification_client.cpp index 95e26f57c..6be570449 100644 --- a/src/viam/examples/mlmodel/example_audio_classification_client.cpp +++ b/src/viam/examples/mlmodel/example_audio_classification_client.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include @@ -78,6 +79,10 @@ constexpr char kRobotConfigTemplate[] = R"( } // namespace int main(int argc, char* argv[]) try { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + viam::sdk::Instance inst; + // Build up our command line options. The example operates in two // modes. In the "--generate" mode, it takes command line // parameters needed to satisfy the interpolation points in the diff --git a/src/viam/examples/modules/complex/client.cpp b/src/viam/examples/modules/complex/client.cpp index ac85c9f00..4c26f055f 100644 --- a/src/viam/examples/modules/complex/client.cpp +++ b/src/viam/examples/modules/complex/client.cpp @@ -11,6 +11,7 @@ #include #include +#include #include #include #include @@ -21,6 +22,10 @@ using namespace viam::sdk; int main() { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + Instance inst; + const char* uri = "http://localhost:8080/"; // replace with your URI if connecting securely DialOptions dial_options; dial_options.set_allow_insecure_downgrade(true); // set to false if connecting securely diff --git a/src/viam/examples/modules/complex/main.cpp b/src/viam/examples/modules/complex/main.cpp index da80453e6..c6a1e7532 100644 --- a/src/viam/examples/modules/complex/main.cpp +++ b/src/viam/examples/modules/complex/main.cpp @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -24,11 +25,15 @@ using namespace viam::sdk; int main(int argc, char** argv) { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + Instance inst; + Model mybase_model("viam", "base", "mybase"); // Make sure to explicity register resources with custom APIs. - Registry::get().register_resource_server(); - Registry::get().register_resource_server(); + inst.registry()->register_resource_server(); + inst.registry()->register_resource_server(); std::shared_ptr mybase_mr = std::make_shared( API::get(), diff --git a/src/viam/examples/modules/complex/test_complex_module.cpp b/src/viam/examples/modules/complex/test_complex_module.cpp index 14d68a8cf..3f4d4ee3d 100644 --- a/src/viam/examples/modules/complex/test_complex_module.cpp +++ b/src/viam/examples/modules/complex/test_complex_module.cpp @@ -24,8 +24,9 @@ using namespace viam::sdktests; struct RegisterGizmoAndSummationFixture { RegisterGizmoAndSummationFixture() { - Registry::get().register_resource(); - Registry::get().register_resource(); + auto* registry = Instance::current().registry(); + registry->register_resource(); + registry->register_resource(); } // Test teardown is a noop; diff --git a/src/viam/examples/modules/simple/client.cpp b/src/viam/examples/modules/simple/client.cpp index fcaab5eff..09e510636 100644 --- a/src/viam/examples/modules/simple/client.cpp +++ b/src/viam/examples/modules/simple/client.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include #include @@ -10,6 +11,8 @@ using namespace viam::sdk; int main() { + Instance inst; + const char* uri = "http://localhost:8080/"; // replace with your URI if connecting securely DialOptions dial_options; dial_options.set_allow_insecure_downgrade(true); // set to false if connecting securely diff --git a/src/viam/examples/modules/simple/main.cpp b/src/viam/examples/modules/simple/main.cpp index cfd783f66..0df75464b 100644 --- a/src/viam/examples/modules/simple/main.cpp +++ b/src/viam/examples/modules/simple/main.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -75,6 +76,8 @@ ProtoStruct MySensor::get_readings(const ProtoStruct&) { } int main(int argc, char** argv) try { + Instance inst; + Model mysensor_model("viam", "sensor", "mysensor"); std::shared_ptr mr = std::make_shared( diff --git a/src/viam/examples/modules/tflite/main.cpp b/src/viam/examples/modules/tflite/main.cpp index 44cd8e284..e1c806354 100644 --- a/src/viam/examples/modules/tflite/main.cpp +++ b/src/viam/examples/modules/tflite/main.cpp @@ -23,6 +23,7 @@ #include +#include #include #include #include @@ -723,6 +724,10 @@ class MLModelServiceTFLite : public vsdk::MLModelService, }; int serve(const std::string& socket_path) try { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. + vsdk::Instance inst; + // Create a new model registration for the service. auto module_registration = std::make_shared( // Identify that this resource offers the MLModelService API @@ -737,10 +742,10 @@ int serve(const std::string& socket_path) try { }); // Register the newly created registration with the Registry. - vsdk::Registry::register_model(module_registration); + inst.registry()->register_model(module_registration); // Construct the module service and tell it where to place the socket path. - auto module_service = std::make_shared(socket_path); + auto module_service = std::make_shared(socket_path, inst.registry()); // Add the server as providing the API and model declared in the // registration. diff --git a/src/viam/examples/motor/example_motor.cpp b/src/viam/examples/motor/example_motor.cpp index c4d8023d5..3d5a640ef 100644 --- a/src/viam/examples/motor/example_motor.cpp +++ b/src/viam/examples/motor/example_motor.cpp @@ -6,6 +6,7 @@ #include #include +#include #include #include #include @@ -27,6 +28,11 @@ int main() { namespace vs = ::viam::sdk; try { + // Every Viam C++ SDK program must have one and only one Instance object which is created + // before any other C++ SDK objects and stays alive until all Viam C++ SDK objects are + // destroyed. + vs::Instance inst; + // 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"); From 60183237def4a3aff5c2163efd31ce86630b45c6 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Thu, 27 Feb 2025 14:26:16 -0500 Subject: [PATCH 14/31] make instance more of a black box --- src/viam/examples/modules/complex/main.cpp | 4 ++-- .../examples/modules/complex/test_complex_module.cpp | 6 +++--- src/viam/examples/modules/tflite/main.cpp | 2 +- src/viam/sdk/common/instance.cpp | 9 +-------- src/viam/sdk/common/instance.hpp | 6 ++---- src/viam/sdk/registry/registry.cpp | 4 ++-- src/viam/sdk/tests/mocks/mock_robot.cpp | 2 +- 7 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/viam/examples/modules/complex/main.cpp b/src/viam/examples/modules/complex/main.cpp index c6a1e7532..253e69d78 100644 --- a/src/viam/examples/modules/complex/main.cpp +++ b/src/viam/examples/modules/complex/main.cpp @@ -32,8 +32,8 @@ int main(int argc, char** argv) { Model mybase_model("viam", "base", "mybase"); // Make sure to explicity register resources with custom APIs. - inst.registry()->register_resource_server(); - inst.registry()->register_resource_server(); + Registry::get().register_resource_server(); + Registry::get().register_resource_server(); std::shared_ptr mybase_mr = std::make_shared( API::get(), diff --git a/src/viam/examples/modules/complex/test_complex_module.cpp b/src/viam/examples/modules/complex/test_complex_module.cpp index 3f4d4ee3d..4a178271f 100644 --- a/src/viam/examples/modules/complex/test_complex_module.cpp +++ b/src/viam/examples/modules/complex/test_complex_module.cpp @@ -24,9 +24,9 @@ using namespace viam::sdktests; struct RegisterGizmoAndSummationFixture { RegisterGizmoAndSummationFixture() { - auto* registry = Instance::current().registry(); - registry->register_resource(); - registry->register_resource(); + auto& registry = Registry::get(); + registry.register_resource(); + registry.register_resource(); } // Test teardown is a noop; diff --git a/src/viam/examples/modules/tflite/main.cpp b/src/viam/examples/modules/tflite/main.cpp index e1c806354..c1224266f 100644 --- a/src/viam/examples/modules/tflite/main.cpp +++ b/src/viam/examples/modules/tflite/main.cpp @@ -742,7 +742,7 @@ int serve(const std::string& socket_path) try { }); // Register the newly created registration with the Registry. - inst.registry()->register_model(module_registration); + Registry::get().register_model(module_registration); // Construct the module service and tell it where to place the socket path. auto module_service = std::make_shared(socket_path, inst.registry()); diff --git a/src/viam/sdk/common/instance.cpp b/src/viam/sdk/common/instance.cpp index c366ad6c6..bc52367f6 100644 --- a/src/viam/sdk/common/instance.cpp +++ b/src/viam/sdk/common/instance.cpp @@ -1,6 +1,7 @@ #include #include +#include #include #include @@ -17,10 +18,6 @@ std::atomic current_instance{nullptr}; } // namespace -struct Instance::Impl { - Registry registry; -}; - Instance::Instance() { Instance* expected = nullptr; @@ -52,9 +49,5 @@ Instance& Instance::current() { return *current; } -Registry* Instance::registry() { - return &(impl_->registry); -} - } // namespace sdk } // namespace viam diff --git a/src/viam/sdk/common/instance.hpp b/src/viam/sdk/common/instance.hpp index c1ab0961f..1c97a2479 100644 --- a/src/viam/sdk/common/instance.hpp +++ b/src/viam/sdk/common/instance.hpp @@ -5,8 +5,6 @@ namespace viam { namespace sdk { -class Registry; - /// @brief Instance management for Viam C++ SDK applications. /// This is a single instance class which is responsible for global setup and teardown related to /// the SDK. An Instance must be constructed before doing anything else in a program, and it must @@ -19,9 +17,9 @@ class Instance { static Instance& current(); - Registry* registry(); - private: + friend class Registry; + struct Impl; std::unique_ptr impl_; }; diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 1ee9b5533..b1e8fb604 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -12,7 +12,7 @@ #include #include -#include +#include #include #include #include @@ -92,7 +92,7 @@ const Model& ModelRegistration::model() const { }; Registry& Registry::get() { - return *Instance::current().registry(); + return Instance::current().impl_->registry; } void Registry::register_model(std::shared_ptr resource) { diff --git a/src/viam/sdk/tests/mocks/mock_robot.cpp b/src/viam/sdk/tests/mocks/mock_robot.cpp index 5dc03fcc2..ce9a62884 100644 --- a/src/viam/sdk/tests/mocks/mock_robot.cpp +++ b/src/viam/sdk/tests/mocks/mock_robot.cpp @@ -26,7 +26,7 @@ std::vector registered_models_for_resource(const std::shared_ptr std::string resource_type; std::string resource_subtype; std::vector resource_names; - for (const auto& kv : Instance::current().registry()->registered_models()) { + for (const auto& kv : Registry::get().registered_models()) { const std::shared_ptr reg = kv.second; if (reg->api() == resource->api()) { resource_type = reg->api().resource_type(); From 5535616d43a2fabe14e9c5a5a7d5b5718252a924 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Thu, 27 Feb 2025 14:40:39 -0500 Subject: [PATCH 15/31] add impl instance to git --- src/viam/sdk/common/private/instance.hpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 src/viam/sdk/common/private/instance.hpp diff --git a/src/viam/sdk/common/private/instance.hpp b/src/viam/sdk/common/private/instance.hpp new file mode 100644 index 000000000..b1728de8c --- /dev/null +++ b/src/viam/sdk/common/private/instance.hpp @@ -0,0 +1,14 @@ +#pragma once + +#include +#include + +namespace viam { +namespace sdk { + +struct Instance::Impl { + Registry registry; +}; + +} // namespace sdk +} // namespace viam From eeac195427f24641a37fceccaf4aa87fd4df6385 Mon Sep 17 00:00:00 2001 From: lia <167905060+lia-viam@users.noreply.github.com> Date: Mon, 3 Mar 2025 10:05:20 -0500 Subject: [PATCH 16/31] Add instance comment boilerplate Co-authored-by: Ethan --- src/viam/examples/modules/simple/client.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/viam/examples/modules/simple/client.cpp b/src/viam/examples/modules/simple/client.cpp index 09e510636..f08410336 100644 --- a/src/viam/examples/modules/simple/client.cpp +++ b/src/viam/examples/modules/simple/client.cpp @@ -11,6 +11,8 @@ using namespace viam::sdk; int main() { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. Instance inst; const char* uri = "http://localhost:8080/"; // replace with your URI if connecting securely From 7560f6d0c965939d5ae388bd49766aa268c19e67 Mon Sep 17 00:00:00 2001 From: lia <167905060+lia-viam@users.noreply.github.com> Date: Mon, 3 Mar 2025 10:05:49 -0500 Subject: [PATCH 17/31] Add instance comment boilerplate Co-authored-by: Ethan --- src/viam/examples/modules/simple/main.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/viam/examples/modules/simple/main.cpp b/src/viam/examples/modules/simple/main.cpp index 0df75464b..e85c2fe3a 100644 --- a/src/viam/examples/modules/simple/main.cpp +++ b/src/viam/examples/modules/simple/main.cpp @@ -76,6 +76,8 @@ ProtoStruct MySensor::get_readings(const ProtoStruct&) { } int main(int argc, char** argv) try { + // Every Viam C++ SDK program must have one and only one Instance object which is created before + // any other C++ SDK objects and stays alive until all Viam C++ SDK objects are destroyed. Instance inst; Model mysensor_model("viam", "sensor", "mysensor"); From 4e25d35b30bfaa3fae2452576e23c32da9c57690 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Mon, 3 Mar 2025 14:52:30 -0500 Subject: [PATCH 18/31] remove old ctor arg --- src/viam/examples/modules/tflite/main.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/viam/examples/modules/tflite/main.cpp b/src/viam/examples/modules/tflite/main.cpp index c1224266f..65ca8c191 100644 --- a/src/viam/examples/modules/tflite/main.cpp +++ b/src/viam/examples/modules/tflite/main.cpp @@ -745,7 +745,7 @@ int serve(const std::string& socket_path) try { Registry::get().register_model(module_registration); // Construct the module service and tell it where to place the socket path. - auto module_service = std::make_shared(socket_path, inst.registry()); + auto module_service = std::make_shared(socket_path); // Add the server as providing the API and model declared in the // registration. From 55baa6c00d4d3de8eeaaeb2befb13423db23c143 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Mon, 3 Mar 2025 14:53:26 -0500 Subject: [PATCH 19/31] prohibit multi instance --- src/viam/sdk/common/instance.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/viam/sdk/common/instance.hpp b/src/viam/sdk/common/instance.hpp index 1c97a2479..64bb430c2 100644 --- a/src/viam/sdk/common/instance.hpp +++ b/src/viam/sdk/common/instance.hpp @@ -8,8 +8,8 @@ namespace sdk { /// @brief Instance management for Viam C++ SDK applications. /// This is a single instance class which is responsible for global setup and teardown related to /// the SDK. An Instance must be constructed before doing anything else in a program, and it must -/// remain alive in a valid state for the duration of the program. Creating multiple overlapping -/// Instance objects is an error. +/// remain alive in a valid state for the duration of the program. Creating multiple Instance +/// objects in the same program is an error. class Instance { public: Instance(); From 10cd711441b03ea12bea259ba27013d622a0a8a3 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Mon, 3 Mar 2025 14:54:54 -0500 Subject: [PATCH 20/31] remove initialized flag --- src/viam/sdk/registry/registry.cpp | 7 ------- src/viam/sdk/registry/registry.hpp | 3 ++- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index b1e8fb604..15b1f118b 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -219,13 +219,6 @@ void Registry::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."; - return; - } - initialized_ = true; register_resources(); grpc::reflection::InitProtoReflectionServerBuilderPlugin(); } diff --git a/src/viam/sdk/registry/registry.hpp b/src/viam/sdk/registry/registry.hpp index f09ee2d62..c1a5055a3 100644 --- a/src/viam/sdk/registry/registry.hpp +++ b/src/viam/sdk/registry/registry.hpp @@ -194,8 +194,9 @@ class Registry { void initialize(); mutable std::mutex lock_; - bool initialized_{false}; + std::unordered_map> resources_; + std::unordered_map> client_apis_; std::unordered_map> server_apis_; From c74542296e2513e09281450fdb6e1452b72129a5 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Mon, 3 Mar 2025 14:55:54 -0500 Subject: [PATCH 21/31] meyers singleton registry --- src/viam/sdk/registry/registry.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index 15b1f118b..e538d2ec2 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -92,7 +92,9 @@ const Model& ModelRegistration::model() const { }; Registry& Registry::get() { - return Instance::current().impl_->registry; + static Registry& result = Instance::current().impl_->registry; + + return result; } void Registry::register_model(std::shared_ptr resource) { From 75ab450a7fa122ff93e50ff9226ab9c49f003177 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Mon, 3 Mar 2025 15:13:33 -0500 Subject: [PATCH 22/31] remove registry member and fix member spacing/ordering --- src/viam/sdk/module/service.cpp | 14 ++++++-------- src/viam/sdk/module/service.hpp | 2 -- src/viam/sdk/robot/client.cpp | 6 +++--- src/viam/sdk/robot/client.hpp | 9 +++++++-- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index d644b5420..911be9bb3 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -57,7 +57,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { std::shared_ptr res; const Dependencies deps = parent.get_dependencies_(&request->dependencies(), cfg.name()); const std::shared_ptr reg = - parent.registry_->lookup_model(cfg.api(), cfg.model()); + Registry::get().lookup_model(cfg.api(), cfg.model()); if (reg) { try { res = reg->construct_resource(deps, cfg); @@ -112,7 +112,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { } const std::shared_ptr reg = - parent.registry_->lookup_model(cfg.name()); + Registry::get().lookup_model(cfg.name()); if (reg) { try { const std::shared_ptr res = reg->construct_resource(deps, cfg); @@ -132,7 +132,7 @@ struct ModuleService::ServiceImpl : viam::module::v1::ModuleService::Service { ResourceConfig cfg = from_proto(proto); const std::shared_ptr reg = - parent.registry_->lookup_model(cfg.api(), cfg.model()); + Registry::get().lookup_model(cfg.api(), cfg.model()); if (!reg) { return grpc::Status(grpc::UNKNOWN, "unable to validate resource " + cfg.resource_name().name() + @@ -215,9 +215,7 @@ std::shared_ptr ModuleService::get_parent_resource_(const Name& name) } ModuleService::ModuleService(std::string addr) - : registry_(&Registry::get()), - module_(std::make_unique(std::move(addr))), - server_(std::make_unique()) { + : module_(std::make_unique(std::move(addr))), server_(std::make_unique()) { impl_ = std::make_unique(*this); } @@ -234,7 +232,7 @@ ModuleService::ModuleService(int argc, set_logger_severity_from_args(argc, argv); for (auto&& mr : registrations) { - registry_->register_model(mr); + Registry::get().register_model(mr); add_model_from_registry(mr->api(), mr->model()); } } @@ -277,7 +275,7 @@ void ModuleService::add_model_from_registry_inlock_(API api, Model model, const std::lock_guard&) { const std::shared_ptr creator = - registry_->lookup_resource_server(api); + Registry::get().lookup_resource_server(api); std::string name; if (creator && creator->service_descriptor()) { name = creator->service_descriptor()->full_name(); diff --git a/src/viam/sdk/module/service.hpp b/src/viam/sdk/module/service.hpp index 95a7ad4f3..799ba1ab5 100644 --- a/src/viam/sdk/module/service.hpp +++ b/src/viam/sdk/module/service.hpp @@ -64,8 +64,6 @@ class ModuleService { std::string const& resource_name); std::shared_ptr get_parent_resource_(const Name& name); - Registry* registry_; - std::mutex lock_; std::unique_ptr module_; std::shared_ptr parent_; diff --git a/src/viam/sdk/robot/client.cpp b/src/viam/sdk/robot/client.cpp index a8cb16c75..6c2f0f0fe 100644 --- a/src/viam/sdk/robot/client.cpp +++ b/src/viam/sdk/robot/client.cpp @@ -178,7 +178,8 @@ void RobotClient::refresh() { // are being properly registered from name.subtype(), or update what we're // using for lookup const std::shared_ptr rs = - registry_->lookup_resource_client({name.namespace_(), name.type(), name.subtype()}); + Registry::get().lookup_resource_client( + {name.namespace_(), name.type(), name.subtype()}); if (rs) { try { const std::shared_ptr rpc_client = @@ -222,8 +223,7 @@ void RobotClient::refresh_every() { }; RobotClient::RobotClient(std::shared_ptr channel) - : registry_(&Registry::get()), - channel_(channel->channel()), + : channel_(channel->channel()), viam_channel_(std::move(channel)), should_close_channel_(false), impl_(std::make_unique(RobotService::NewStub(channel_))) {} diff --git a/src/viam/sdk/robot/client.hpp b/src/viam/sdk/robot/client.hpp index 410abca28..50fda612c 100644 --- a/src/viam/sdk/robot/client.hpp +++ b/src/viam/sdk/robot/client.hpp @@ -147,19 +147,24 @@ class RobotClient { status get_machine_status() const; private: - Registry* registry_; + void refresh_every(); + std::vector> threads_; + std::atomic should_refresh_; unsigned int refresh_interval_; + std::shared_ptr channel_; std::shared_ptr viam_channel_; bool should_close_channel_; + struct impl; std::unique_ptr impl_; + mutable std::mutex lock_; + std::vector resource_names_; ResourceManager resource_manager_; - void refresh_every(); }; } // namespace sdk From fc21c356bf455947c5cd8d3dd68b174b354a5766 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Mon, 3 Mar 2025 15:19:14 -0500 Subject: [PATCH 23/31] remove unused include --- src/viam/sdk/tests/test_utils.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index 26a51444e..11519a551 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -2,7 +2,6 @@ #include -#include #include #include #include From f02ce0f75d81baa28d62e88f9771e46132cab562 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 12 Mar 2025 13:44:21 -0400 Subject: [PATCH 24/31] reorder includes --- src/viam/sdk/tests/test_mlmodel.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/viam/sdk/tests/test_mlmodel.cpp b/src/viam/sdk/tests/test_mlmodel.cpp index 5d2db8010..8bc6a62f0 100644 --- a/src/viam/sdk/tests/test_mlmodel.cpp +++ b/src/viam/sdk/tests/test_mlmodel.cpp @@ -12,20 +12,19 @@ // See the License for the specific language governing permissions and // limitations under the License. +#define BOOST_TEST_MODULE test module test_mlmodel #include #include #include #include +#include #include #include #include -#define BOOST_TEST_MODULE test module test_mlmodel -#include - namespace viam { namespace sdk { From e80bf807136a7b956d1744ac204e27fe6fd39f14 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 12 Mar 2025 13:45:01 -0400 Subject: [PATCH 25/31] conditionally find unit test framework and link it to tests --- CMakeLists.txt | 6 +++++- src/viam/sdk/tests/CMakeLists.txt | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4bebd7564..c025261b6 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -324,7 +324,11 @@ set(VIAMCPPSDK_XTL_VERSION_MINIMUM 0.7.2) set(VIAMCPPSDK_XTENSOR_VERSION_MINIMUM 0.24.3) # Time to find `BOOST`. -find_package(Boost ${VIAMCPPSDK_BOOST_VERSION_MINIMUM} REQUIRED COMPONENTS headers log program_options) +if (VIAMCPPSDK_BUILD_TESTS) + set(VIAMCPPSDK_BOOST_TEST "unit_test_framework") +endif() + +find_package(Boost ${VIAMCPPSDK_BOOST_VERSION_MINIMUM} REQUIRED COMPONENTS headers log program_options ${VIAMCPPSDK_BOOST_TEST}) # Time to find `protobuf` and `gRPC[++]`. Normally this would just be # something like `find_package(gRPC CONFIG REQUIRED)`, and diff --git a/src/viam/sdk/tests/CMakeLists.txt b/src/viam/sdk/tests/CMakeLists.txt index de1fbf0b7..fa2d1560d 100644 --- a/src/viam/sdk/tests/CMakeLists.txt +++ b/src/viam/sdk/tests/CMakeLists.txt @@ -47,6 +47,7 @@ target_include_directories(viamsdk_test target_link_libraries(viamsdk_test PUBLIC viam-cpp-sdk::viamsdk + PUBLIC Boost::unit_test_framework ) viamcppsdk_link_viam_api(viamsdk_test PUBLIC) From 185121ae13f80aa0c84858e7130349b7a46afd99 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 12 Mar 2025 13:45:44 -0400 Subject: [PATCH 26/31] registry get cannot create instance by default --- src/viam/sdk/common/instance.cpp | 10 +++++++--- src/viam/sdk/common/instance.hpp | 11 ++++++++++- src/viam/sdk/registry/registry.cpp | 2 +- src/viam/sdk/tests/test_utils.cpp | 2 +- src/viam/sdk/tests/test_utils.hpp | 12 ++++++++++++ 5 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/viam/sdk/common/instance.cpp b/src/viam/sdk/common/instance.cpp index bc52367f6..86443f458 100644 --- a/src/viam/sdk/common/instance.cpp +++ b/src/viam/sdk/common/instance.cpp @@ -34,10 +34,14 @@ Instance::~Instance() { impl_.reset(); } -Instance& Instance::current() { +Instance& Instance::current(Instance::Creation creation) { if (!current_instance.load()) { - // This variable declaration calls the default ctor, storing a current instance. - static Instance inst; // NOLINT (misc-const-correctness) + if (creation == Creation::if_needed) { + // This variable declaration calls the default ctor, storing a current instance. + static Instance inst; // NOLINT (misc-const-correctness) + } else { + throw Exception("Instance has not yet been created"); + } } Instance* current = current_instance.load(); diff --git a/src/viam/sdk/common/instance.hpp b/src/viam/sdk/common/instance.hpp index 64bb430c2..1dd0901df 100644 --- a/src/viam/sdk/common/instance.hpp +++ b/src/viam/sdk/common/instance.hpp @@ -12,10 +12,19 @@ namespace sdk { /// objects in the same program is an error. class Instance { public: + /// @brief Enumeration for creation behavior of @ref current + enum class Creation { + open_existing, ///< Instance must already exist + if_needed ///< Use existing instance if present, else create one. + }; + Instance(); ~Instance(); - static Instance& current(); + /// @brief Get the current Instance according to the Creation behavior. + /// Calling current(Creation::open_existing) when an instance has not yet been constructed is an + /// error. + static Instance& current(Creation); private: friend class Registry; diff --git a/src/viam/sdk/registry/registry.cpp b/src/viam/sdk/registry/registry.cpp index e538d2ec2..de74d314d 100644 --- a/src/viam/sdk/registry/registry.cpp +++ b/src/viam/sdk/registry/registry.cpp @@ -92,7 +92,7 @@ const Model& ModelRegistration::model() const { }; Registry& Registry::get() { - static Registry& result = Instance::current().impl_->registry; + static Registry& result = Instance::current(Instance::Creation::open_existing).impl_->registry; return result; } diff --git a/src/viam/sdk/tests/test_utils.cpp b/src/viam/sdk/tests/test_utils.cpp index 0ce5c83ca..c83e59580 100644 --- a/src/viam/sdk/tests/test_utils.cpp +++ b/src/viam/sdk/tests/test_utils.cpp @@ -10,8 +10,8 @@ #include namespace viam { -namespace sdktests { +namespace sdktests { using namespace viam::sdk; ProtoStruct fake_map() { diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index 11519a551..201451188 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -2,6 +2,9 @@ #include +#include + +#include #include #include #include @@ -10,6 +13,15 @@ namespace viam { namespace sdktests { +struct GlobalFixture { + GlobalFixture() { + BOOST_TEST_MESSAGE("Creating instance"); + (void)sdk::Instance::current(sdk::Instance::Creation::if_needed); + } +}; + +BOOST_TEST_GLOBAL_FIXTURE(GlobalFixture); + using namespace viam::sdk; ProtoStruct fake_map(); From 7a747ec8f99fd9d529db8c37f3713b9d7a6bc7dc Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 12 Mar 2025 13:48:39 -0400 Subject: [PATCH 27/31] instance models fixture --- src/viam/sdk/tests/test_utils.hpp | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index 201451188..43825b8e2 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -13,16 +13,8 @@ namespace viam { namespace sdktests { -struct GlobalFixture { - GlobalFixture() { - BOOST_TEST_MESSAGE("Creating instance"); - (void)sdk::Instance::current(sdk::Instance::Creation::if_needed); - } -}; - -BOOST_TEST_GLOBAL_FIXTURE(GlobalFixture); - using namespace viam::sdk; +BOOST_TEST_GLOBAL_FIXTURE(Instance); ProtoStruct fake_map(); std::vector fake_geometries(); From 9c4dc97111a2ce43bd8f0f917e0b3566601e0bdd Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 12 Mar 2025 14:05:15 -0400 Subject: [PATCH 28/31] create if needed instance in fixture --- src/viam/sdk/tests/test_utils.hpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/viam/sdk/tests/test_utils.hpp b/src/viam/sdk/tests/test_utils.hpp index 43825b8e2..bb69e2499 100644 --- a/src/viam/sdk/tests/test_utils.hpp +++ b/src/viam/sdk/tests/test_utils.hpp @@ -13,8 +13,15 @@ namespace viam { namespace sdktests { +struct GlobalFixture { + GlobalFixture() { + (void)sdk::Instance::current(sdk::Instance::Creation::if_needed); + } +}; + +BOOST_TEST_GLOBAL_FIXTURE(GlobalFixture); + using namespace viam::sdk; -BOOST_TEST_GLOBAL_FIXTURE(Instance); ProtoStruct fake_map(); std::vector fake_geometries(); From 7409dc78319586f9141d4070f61b1f30f0b95cab Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Wed, 12 Mar 2025 14:08:11 -0400 Subject: [PATCH 29/31] revert namespace ws --- src/viam/sdk/tests/test_utils.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/viam/sdk/tests/test_utils.cpp b/src/viam/sdk/tests/test_utils.cpp index c83e59580..9e76e2568 100644 --- a/src/viam/sdk/tests/test_utils.cpp +++ b/src/viam/sdk/tests/test_utils.cpp @@ -10,7 +10,6 @@ #include namespace viam { - namespace sdktests { using namespace viam::sdk; From c4aaadfa92bccfd8dcade7d4a45587177f5e3cb4 Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Thu, 13 Mar 2025 13:02:19 -0400 Subject: [PATCH 30/31] code review: update doc comment --- src/viam/sdk/common/instance.hpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/viam/sdk/common/instance.hpp b/src/viam/sdk/common/instance.hpp index 1dd0901df..096815943 100644 --- a/src/viam/sdk/common/instance.hpp +++ b/src/viam/sdk/common/instance.hpp @@ -22,8 +22,8 @@ class Instance { ~Instance(); /// @brief Get the current Instance according to the Creation behavior. - /// Calling current(Creation::open_existing) when an instance has not yet been constructed is an - /// error. + /// @throws an @ref Exception if current(Creation::open_existing) is called when an instance has + /// not yet been constructed. static Instance& current(Creation); private: From d2ae2d71cb61d44789cb17ee9b9f881bb414a68d Mon Sep 17 00:00:00 2001 From: Lia Stratopoulos <167905060+lia-viam@users.noreply.github.com> Date: Thu, 13 Mar 2025 13:02:43 -0400 Subject: [PATCH 31/31] code review: include order --- src/viam/sdk/common/instance.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/viam/sdk/common/instance.cpp b/src/viam/sdk/common/instance.cpp index 86443f458..e3a84ea65 100644 --- a/src/viam/sdk/common/instance.cpp +++ b/src/viam/sdk/common/instance.cpp @@ -1,11 +1,11 @@ #include +#include + #include #include #include -#include - namespace viam { namespace sdk {