-
Notifications
You must be signed in to change notification settings - Fork 26
RSDK-3030 - Refactor registry #208
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0c1785b
17eedd3
78a677a
6eb7767
15f5070
eaad4b0
58b2515
a0c47e4
e084c1a
bb9b55f
fbccc83
44a965b
1902a0f
5b5530c
a5eace3
f938ec5
a70d01b
80cdd4d
6b6e0e4
188786c
8adda84
ad47447
baad2ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,36 +28,29 @@ | |
| using namespace viam::sdk; | ||
|
|
||
| int main(int argc, char** argv) { | ||
| API base_api = API::get<Base>(); | ||
| Model mybase_model("viam", "base", "mybase"); | ||
|
|
||
| // Make sure to explicity register resources with custom APIs. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the "must be done in |
||
| Registry::register_resource_server<GizmoServer>(); | ||
| Registry::register_resource_server<SummationServer>(); | ||
|
|
||
| std::shared_ptr<ModelRegistration> mybase_mr = std::make_shared<ModelRegistration>( | ||
| base_api, | ||
| API::get<Base>(), | ||
| mybase_model, | ||
| [](Dependencies deps, ResourceConfig cfg) { return std::make_unique<MyBase>(deps, cfg); }, | ||
| MyBase::validate); | ||
|
|
||
| API gizmo_api = API::get<Gizmo>(); | ||
| Model mygizmo_model("viam", "gizmo", "mygizmo"); | ||
| // Make sure to explicity register resources with custom APIs. Note that | ||
| // this must be done in `main` and not in resource implementation files due | ||
| // to order of static initialization. | ||
| Registry::register_resource(gizmo_api, Gizmo::resource_registration()); | ||
| std::shared_ptr<ModelRegistration> mygizmo_mr = std::make_shared<ModelRegistration>( | ||
| gizmo_api, | ||
| API::get<Gizmo>(), | ||
| mygizmo_model, | ||
| [](Dependencies deps, ResourceConfig cfg) { return std::make_unique<MyGizmo>(deps, cfg); }, | ||
| MyGizmo::validate); | ||
|
|
||
| API summation_api = API::get<Summation>(); | ||
| Model mysummation_model("viam", "summation", "mysummation"); | ||
| // Make sure to explicity register resources with custom APIs. Note that | ||
| // this must be done in `main` and not in resource implementation files due | ||
| // to order of static initialization. | ||
| Registry::register_resource(summation_api, Summation::resource_registration()); | ||
|
|
||
| std::shared_ptr<ModelRegistration> mysummation_mr = std::make_shared<ModelRegistration>( | ||
| summation_api, mysummation_model, [](Dependencies deps, ResourceConfig cfg) { | ||
| API::get<Summation>(), mysummation_model, [](Dependencies deps, ResourceConfig cfg) { | ||
| return std::make_unique<MySummation>(deps, cfg); | ||
| }); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,13 +36,6 @@ endif() | |
|
|
||
| target_sources(viamsdk | ||
| PRIVATE | ||
| # TODO(RSDK-1742): we have to put `registry` up top here because we need the | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| # registry types to be defined first, before anything tries to init them. | ||
| # this obviously isn't great. it breaks up stylistic ordering and is brittle | ||
| # when we need to add updates. we should refactor to make this unnecessary. | ||
| # consider making all necessary runtime values a single `context` that has to | ||
| # be initialized within main before anything else happens? | ||
| registry/registry.cpp | ||
| common/client_helper.cpp | ||
| common/linear_algebra.cpp | ||
| common/pose.cpp | ||
|
|
@@ -87,6 +80,7 @@ target_sources(viamsdk | |
| module/service.cpp | ||
| module/signal_manager.cpp | ||
| referenceframe/frame.cpp | ||
| registry/registry.cpp | ||
| resource/resource.cpp | ||
| resource/resource_api.cpp | ||
| resource/resource_manager.cpp | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,27 +11,13 @@ | |
| #include <viam/sdk/common/proto_type.hpp> | ||
| #include <viam/sdk/common/utils.hpp> | ||
| #include <viam/sdk/config/resource.hpp> | ||
| #include <viam/sdk/registry/registry.hpp> | ||
| #include <viam/sdk/resource/resource_manager.hpp> | ||
| #include <viam/sdk/resource/stoppable.hpp> | ||
|
|
||
| namespace viam { | ||
| namespace sdk { | ||
|
|
||
| /// @defgroup Base Classes related to the Base component. | ||
|
|
||
| /// @class BaseRegistration | ||
| /// @brief Defines a `ResourceRegistration` for the `Base` component. | ||
| /// @ingroup Base | ||
| class BaseRegistration : public ResourceRegistration { | ||
| public: | ||
| explicit BaseRegistration(const google::protobuf::ServiceDescriptor* service_descriptor); | ||
| std::shared_ptr<ResourceServer> create_resource_server(std::shared_ptr<ResourceManager> manager, | ||
| Server& server) override; | ||
| std::shared_ptr<Resource> create_rpc_client(std::string name, | ||
| std::shared_ptr<grpc::Channel> chan) override; | ||
| }; | ||
|
|
||
| /// @class Base base.hpp "components/base/base.hpp" | ||
| /// @brief A `Base` is the platform that the other parts of a mobile robot attach to. | ||
| /// @ingroup Base | ||
|
|
@@ -52,9 +38,6 @@ class Base : public Component, public Stoppable { | |
| friend std::ostream& operator<<(std::ostream& os, const properties& v); | ||
| friend bool operator==(const properties& lhs, const properties& rhs); | ||
|
|
||
| // functions shared across all components | ||
| static std::shared_ptr<ResourceRegistration> resource_registration(); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, I should add that I am happy to see the registration entry points removed from the base classes. It was a little weird that the base classes were responsible for registering the client subclasses and associated servers types. |
||
|
|
||
| /// @brief Move a robot's base in a straight line by a given distance. This method blocks | ||
| /// until completed or cancelled | ||
| /// @param distance_mm Desired travel distance in millimeters | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍