From f235eb316325f930eb4435526fee2457eb9b4b05 Mon Sep 17 00:00:00 2001
From: Ethan Rodkin <ethan@viam.com>
Date: Wed, 14 Feb 2024 15:05:11 -0500
Subject: [PATCH 1/2] reconfigurable base class

---
 .../examples/modules/complex/base/impl.cpp    |  2 +-
 .../examples/modules/complex/base/impl.hpp    |  5 +--
 .../examples/modules/complex/gizmo/impl.cpp   |  2 +-
 .../examples/modules/complex/gizmo/impl.hpp   |  5 +--
 .../modules/complex/summation/impl.cpp        |  2 +-
 .../modules/complex/summation/impl.hpp        |  6 ++--
 src/viam/examples/modules/simple/main.cpp     |  6 ++--
 src/viam/examples/modules/tflite/main.cpp     |  9 ++++--
 src/viam/sdk/CMakeLists.txt                   |  2 ++
 src/viam/sdk/module/service.cpp               |  6 ++--
 src/viam/sdk/resource/reconfigurable.cpp      | 20 ++++++++++++
 src/viam/sdk/resource/reconfigurable.hpp      | 31 +++++++++++++++++++
 src/viam/sdk/resource/resource.cpp            |  2 --
 src/viam/sdk/resource/resource.hpp            |  5 ---
 14 files changed, 77 insertions(+), 26 deletions(-)
 create mode 100644 src/viam/sdk/resource/reconfigurable.cpp
 create mode 100644 src/viam/sdk/resource/reconfigurable.hpp

diff --git a/src/viam/examples/modules/complex/base/impl.cpp b/src/viam/examples/modules/complex/base/impl.cpp
index 0a2517cfe..d31a3ea92 100644
--- a/src/viam/examples/modules/complex/base/impl.cpp
+++ b/src/viam/examples/modules/complex/base/impl.cpp
@@ -34,7 +34,7 @@ std::string find_motor(ResourceConfig cfg, std::string motor_name) {
     return *motor_string;
 }
 
-void MyBase::reconfigure(Dependencies deps, ResourceConfig cfg) {
+void MyBase::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) {
     // Downcast `left` and `right` dependencies to motors.
     auto left = find_motor(cfg, "left");
     auto right = find_motor(cfg, "right");
diff --git a/src/viam/examples/modules/complex/base/impl.hpp b/src/viam/examples/modules/complex/base/impl.hpp
index 378b25469..5bb48ab0b 100644
--- a/src/viam/examples/modules/complex/base/impl.hpp
+++ b/src/viam/examples/modules/complex/base/impl.hpp
@@ -4,6 +4,7 @@
 #include <viam/sdk/components/component.hpp>
 #include <viam/sdk/components/motor/motor.hpp>
 #include <viam/sdk/config/resource.hpp>
+#include <viam/sdk/resource/reconfigurable.hpp>
 #include <viam/sdk/resource/resource.hpp>
 
 using namespace viam::sdk;
@@ -11,12 +12,12 @@ using namespace viam::sdk;
 // `MyBase` inherits from the `Base` class defined in the viam C++ SDK and
 // implements some of the relevant methods along with `reconfigure`. It also
 // specifies a static `validate` method that checks config validity.
-class MyBase : public Base {
+class MyBase : public Base, public Reconfigurable {
    public:
     MyBase(Dependencies deps, ResourceConfig cfg) : Base(cfg.name()) {
         this->reconfigure(deps, cfg);
     };
-    void reconfigure(Dependencies deps, ResourceConfig cfg) override;
+    void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) override;
     static std::vector<std::string> validate(ResourceConfig cfg);
 
     bool is_moving() override;
diff --git a/src/viam/examples/modules/complex/gizmo/impl.cpp b/src/viam/examples/modules/complex/gizmo/impl.cpp
index da0128f50..01a1e7522 100644
--- a/src/viam/examples/modules/complex/gizmo/impl.cpp
+++ b/src/viam/examples/modules/complex/gizmo/impl.cpp
@@ -31,7 +31,7 @@ std::string find_arg1(ResourceConfig cfg) {
     return *arg1_string;
 }
 
-void MyGizmo::reconfigure(Dependencies deps, ResourceConfig cfg) {
+void MyGizmo::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) {
     arg1_ = find_arg1(cfg);
 }
 
diff --git a/src/viam/examples/modules/complex/gizmo/impl.hpp b/src/viam/examples/modules/complex/gizmo/impl.hpp
index 3791ededc..b7799193e 100644
--- a/src/viam/examples/modules/complex/gizmo/impl.hpp
+++ b/src/viam/examples/modules/complex/gizmo/impl.hpp
@@ -3,6 +3,7 @@
 #include <vector>
 
 #include <viam/sdk/common/proto_type.hpp>
+#include <viam/sdk/resource/reconfigurable.hpp>
 
 #include "api.hpp"
 
@@ -11,13 +12,13 @@ using namespace viam::sdk;
 // MyGizmo inherits from the `Gizmo` class defined in `api.hpp` and implements
 // all relevant methods along with `reconfigure`. It also specifies a static
 // `validate` method that checks config validity.
-class MyGizmo : public Gizmo {
+class MyGizmo : public Gizmo, public Reconfigurable {
    public:
     MyGizmo(std::string name, std::string arg1) : Gizmo(std::move(name)), arg1_(std::move(arg1)){};
     MyGizmo(Dependencies deps, ResourceConfig cfg) : Gizmo(cfg.name()) {
         this->reconfigure(deps, cfg);
     };
-    void reconfigure(Dependencies deps, ResourceConfig cfg) override;
+    void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) override;
     static std::vector<std::string> validate(ResourceConfig cfg);
 
     bool do_one(std::string arg1) override;
diff --git a/src/viam/examples/modules/complex/summation/impl.cpp b/src/viam/examples/modules/complex/summation/impl.cpp
index a76eb9a4a..534f4b074 100644
--- a/src/viam/examples/modules/complex/summation/impl.cpp
+++ b/src/viam/examples/modules/complex/summation/impl.cpp
@@ -24,7 +24,7 @@ bool find_subtract(ResourceConfig cfg) {
     return *subtract_bool;
 }
 
-void MySummation::reconfigure(Dependencies deps, ResourceConfig cfg) {
+void MySummation::reconfigure(const Dependencies& deps, const ResourceConfig& cfg) {
     subtract_ = find_subtract(cfg);
 }
 
diff --git a/src/viam/examples/modules/complex/summation/impl.hpp b/src/viam/examples/modules/complex/summation/impl.hpp
index cdef63153..514a19143 100644
--- a/src/viam/examples/modules/complex/summation/impl.hpp
+++ b/src/viam/examples/modules/complex/summation/impl.hpp
@@ -2,20 +2,22 @@
 
 #include <vector>
 
+#include <viam/sdk/resource/reconfigurable.hpp>
+
 #include "api.hpp"
 
 using namespace viam::sdk;
 
 // MySummation inherits from the `Summation` class defined in `api.hpp` and
 // implements all relevant methods along with `reconfigure`.
-class MySummation : public Summation {
+class MySummation : public Summation, public Reconfigurable {
    public:
     MySummation(std::string name, bool subtract)
         : Summation(std::move(name)), subtract_(subtract){};
     MySummation(Dependencies deps, ResourceConfig cfg) : Summation(cfg.name()) {
         this->reconfigure(deps, cfg);
     };
-    void reconfigure(Dependencies deps, ResourceConfig cfg) override;
+    void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) override;
     static std::vector<std::string> validate(ResourceConfig cfg);
 
     double sum(std::vector<double> numbers) override;
diff --git a/src/viam/examples/modules/simple/main.cpp b/src/viam/examples/modules/simple/main.cpp
index 7e74d19eb..6206e425a 100644
--- a/src/viam/examples/modules/simple/main.cpp
+++ b/src/viam/examples/modules/simple/main.cpp
@@ -1,6 +1,5 @@
 #include <iostream>
 #include <memory>
-#include <signal.h>
 #include <sstream>
 
 #include <boost/log/trivial.hpp>
@@ -15,6 +14,7 @@
 #include <viam/sdk/module/module.hpp>
 #include <viam/sdk/module/service.hpp>
 #include <viam/sdk/registry/registry.hpp>
+#include <viam/sdk/resource/reconfigurable.hpp>
 #include <viam/sdk/resource/resource.hpp>
 #include <viam/sdk/rpc/dial.hpp>
 #include <viam/sdk/rpc/server.hpp>
@@ -26,9 +26,9 @@ using namespace viam::sdk;
 // Printer is a modular resource that can print a to_print value to STDOUT when
 // a DoCommand request is received or when reconfiguring. The to_print value
 // must be provided as an attribute in the config.
-class Printer : public GenericService {
+class Printer : public GenericService, public Reconfigurable {
    public:
-    void reconfigure(Dependencies deps, ResourceConfig cfg) {
+    void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) {
         std::cout << "Printer " << Resource::name() << " is reconfiguring" << std::endl;
         for (auto& dep : deps) {
             std::cout << "dependency: " << dep.first.to_string() << std::endl;
diff --git a/src/viam/examples/modules/tflite/main.cpp b/src/viam/examples/modules/tflite/main.cpp
index 9d969b4ae..cb80c2a4d 100644
--- a/src/viam/examples/modules/tflite/main.cpp
+++ b/src/viam/examples/modules/tflite/main.cpp
@@ -30,6 +30,7 @@
 #include <viam/sdk/config/resource.hpp>
 #include <viam/sdk/module/service.hpp>
 #include <viam/sdk/registry/registry.hpp>
+#include <viam/sdk/resource/reconfigurable.hpp>
 #include <viam/sdk/resource/stoppable.hpp>
 #include <viam/sdk/rpc/server.hpp>
 #include <viam/sdk/services/mlmodel/mlmodel.hpp>
@@ -58,7 +59,9 @@ constexpr char service_name[] = "example_mlmodelservice_tflite";
 //      with the model.
 //
 // Any additional configuration fields are ignored.
-class MLModelServiceTFLite : public vsdk::MLModelService, public vsdk::Stoppable {
+class MLModelServiceTFLite : public vsdk::MLModelService,
+                             public vsdk::Stoppable,
+                             public vsdk::Reconfigurable {
     class write_to_tflite_tensor_visitor_;
 
    public:
@@ -93,8 +96,8 @@ class MLModelServiceTFLite : public vsdk::MLModelService, public vsdk::Stoppable
         }
     }
 
-    void reconfigure(vsdk::Dependencies dependencies, vsdk::ResourceConfig configuration) final
-        try {
+    void reconfigure(const vsdk::Dependencies& dependencies,
+                     const vsdk::ResourceConfig& configuration) final try {
         // Care needs to be taken during reconfiguration. The
         // framework does not offer protection against invocation
         // during reconfiguration. Keep all state in a shared_ptr
diff --git a/src/viam/sdk/CMakeLists.txt b/src/viam/sdk/CMakeLists.txt
index 4805e1c07..f5eb7b7cd 100644
--- a/src/viam/sdk/CMakeLists.txt
+++ b/src/viam/sdk/CMakeLists.txt
@@ -82,6 +82,7 @@ target_sources(viamsdk
     module/signal_manager.cpp
     referenceframe/frame.cpp
     registry/registry.cpp
+    resource/reconfigurable.cpp
     resource/resource.cpp
     resource/resource_api.cpp
     resource/resource_manager.cpp
@@ -155,6 +156,7 @@ target_sources(viamsdk
       ../../viam/sdk/module/signal_manager.hpp
       ../../viam/sdk/referenceframe/frame.hpp
       ../../viam/sdk/registry/registry.hpp
+      ../../viam/sdk/resource/reconfigurable.hpp
       ../../viam/sdk/resource/resource.hpp
       ../../viam/sdk/resource/resource_api.hpp
       ../../viam/sdk/resource/resource_manager.hpp
diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp
index 87e6931d7..8b357edf7 100644
--- a/src/viam/sdk/module/service.cpp
+++ b/src/viam/sdk/module/service.cpp
@@ -1,11 +1,8 @@
 #include <viam/sdk/module/service.hpp>
 
 #include <exception>
-#include <fstream>
-#include <iostream>
 #include <memory>
 #include <sstream>
-#include <stdexcept>
 #include <string>
 #include <sys/socket.h>
 #include <sys/stat.h>
@@ -33,6 +30,7 @@
 #include <viam/sdk/config/resource.hpp>
 #include <viam/sdk/module/handler_map.hpp>
 #include <viam/sdk/registry/registry.hpp>
+#include <viam/sdk/resource/reconfigurable.hpp>
 #include <viam/sdk/resource/resource.hpp>
 #include <viam/sdk/resource/resource_api.hpp>
 #include <viam/sdk/resource/resource_manager.hpp>
@@ -121,7 +119,7 @@ ::grpc::Status ModuleService::ReconfigureResource(
                                 " as it doesn't exist.");
     }
     try {
-        res->reconfigure(deps, cfg);
+        Reconfigurable::reconfigure_if_reconfigurable(res, deps, cfg);
         return grpc::Status();
     } catch (const std::exception& exc) {
         return grpc::Status(::grpc::INTERNAL, exc.what());
diff --git a/src/viam/sdk/resource/reconfigurable.cpp b/src/viam/sdk/resource/reconfigurable.cpp
new file mode 100644
index 000000000..ae8683879
--- /dev/null
+++ b/src/viam/sdk/resource/reconfigurable.cpp
@@ -0,0 +1,20 @@
+#include <memory>
+#include <viam/sdk/resource/reconfigurable.hpp>
+
+namespace viam {
+namespace sdk {
+
+Reconfigurable::~Reconfigurable() = default;
+Reconfigurable::Reconfigurable() = default;
+
+void Reconfigurable::reconfigure_if_reconfigurable(const std::shared_ptr<Resource>& resource,
+                                                   const Dependencies& deps,
+                                                   const ResourceConfig& cfg) {
+    auto reconfigurable_res = std::dynamic_pointer_cast<Reconfigurable>(resource);
+    if (reconfigurable_res) {
+        reconfigurable_res->reconfigure(deps, cfg);
+    }
+}
+
+}  // namespace sdk
+}  // namespace viam
diff --git a/src/viam/sdk/resource/reconfigurable.hpp b/src/viam/sdk/resource/reconfigurable.hpp
new file mode 100644
index 000000000..013bf10e1
--- /dev/null
+++ b/src/viam/sdk/resource/reconfigurable.hpp
@@ -0,0 +1,31 @@
+#pragma once
+
+#include <viam/sdk/config/resource.hpp>
+#include <viam/sdk/resource/resource.hpp>
+
+namespace viam {
+namespace sdk {
+
+class Reconfigurable {
+   public:
+    virtual ~Reconfigurable();
+
+    /// @brief Reconfigures a resource.
+    /// @param deps Dependencies of the resource.
+    /// @param cfg The resource's config.
+    virtual void reconfigure(const Dependencies& deps, const ResourceConfig& cfg) = 0;
+
+    /// @brief Reconfigures a resource if it is Reconfigurable.
+    /// @param resource the Resource to reconfigure.
+    /// @param deps Dependencies of the resource.
+    /// @param cfg The resource's config.
+    static void reconfigure_if_reconfigurable(const std::shared_ptr<Resource>& resource,
+                                              const Dependencies& deps,
+                                              const ResourceConfig& cfg);
+
+   protected:
+    explicit Reconfigurable();
+};
+
+}  // namespace sdk
+}  // namespace viam
diff --git a/src/viam/sdk/resource/resource.cpp b/src/viam/sdk/resource/resource.cpp
index adbd6019f..e4ae1664e 100644
--- a/src/viam/sdk/resource/resource.cpp
+++ b/src/viam/sdk/resource/resource.cpp
@@ -19,8 +19,6 @@ std::string Resource::name() const {
     return name_;
 }
 
-void Resource::reconfigure(Dependencies deps, ResourceConfig cfg){};
-
 ResourceName Resource::get_resource_name(std::string name) const {
     ResourceName r;
     *r.mutable_namespace_() = kRDK;
diff --git a/src/viam/sdk/resource/resource.hpp b/src/viam/sdk/resource/resource.hpp
index eacfd2126..fbb194029 100644
--- a/src/viam/sdk/resource/resource.hpp
+++ b/src/viam/sdk/resource/resource.hpp
@@ -25,11 +25,6 @@ class Resource {
     /// @brief Returns a `ResourceName` for a particular resource name.
     virtual viam::common::v1::ResourceName get_resource_name(std::string name) const;
 
-    /// @brief Reconfigures a resource.
-    /// @param deps Dependencies of the resource.
-    /// @param cfg The resource's config.
-    virtual void reconfigure(Dependencies deps, ResourceConfig cfg);
-
     /// @brief Return the resource's name.
     virtual std::string name() const;
 

From f443995bb93147e16ba37ed06dc9f02d9ec80915 Mon Sep 17 00:00:00 2001
From: Ethan Rodkin <ethan@viam.com>
Date: Thu, 22 Feb 2024 10:42:54 -0500
Subject: [PATCH 2/2] header cleanup

---
 src/viam/sdk/resource/reconfigurable.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/viam/sdk/resource/reconfigurable.cpp b/src/viam/sdk/resource/reconfigurable.cpp
index ae8683879..3f65ba516 100644
--- a/src/viam/sdk/resource/reconfigurable.cpp
+++ b/src/viam/sdk/resource/reconfigurable.cpp
@@ -1,6 +1,7 @@
-#include <memory>
 #include <viam/sdk/resource/reconfigurable.hpp>
 
+#include <memory>
+
 namespace viam {
 namespace sdk {