From afe1d683162467481ab0c1843bce994ebbc12105 Mon Sep 17 00:00:00 2001
From: Benji Rewis <benji.rewis@viam.com>
Date: Wed, 29 Nov 2023 16:38:11 +0000
Subject: [PATCH] RSDK-5881 Add Stoppable class

---
 .../examples/modules/complex/gizmo/api.cpp    |  2 --
 .../examples/modules/complex/gizmo/api.hpp    |  1 -
 .../modules/complex/summation/api.cpp         |  2 --
 .../modules/complex/summation/api.hpp         |  1 -
 src/viam/examples/modules/tflite/main.cpp     |  2 +-
 src/viam/sdk/components/base/base.hpp         |  2 +-
 src/viam/sdk/components/board/client.cpp      |  2 --
 src/viam/sdk/components/board/client.hpp      |  1 -
 src/viam/sdk/components/camera/client.cpp     |  2 --
 src/viam/sdk/components/camera/client.hpp     |  1 -
 src/viam/sdk/components/encoder/client.cpp    |  2 --
 src/viam/sdk/components/encoder/client.hpp    |  1 -
 src/viam/sdk/components/generic/client.cpp    |  2 --
 src/viam/sdk/components/generic/client.hpp    |  1 -
 src/viam/sdk/components/motor/motor.hpp       |  2 +-
 .../sdk/components/movement_sensor/client.cpp |  2 --
 .../sdk/components/movement_sensor/client.hpp |  1 -
 .../sdk/components/power_sensor/client.cpp    |  2 --
 .../sdk/components/power_sensor/client.hpp    |  1 -
 src/viam/sdk/components/sensor/client.cpp     |  2 --
 src/viam/sdk/components/sensor/client.hpp     |  1 -
 src/viam/sdk/module/service.cpp               |  4 +--
 src/viam/sdk/resource/resource.cpp            |  7 ++++++
 src/viam/sdk/resource/resource.hpp            | 25 ++++++++++++-------
 src/viam/sdk/robot/service.cpp                |  6 ++---
 src/viam/sdk/services/mlmodel/mlmodel.hpp     | 11 +++++++-
 src/viam/sdk/services/motion/client.cpp       |  2 --
 src/viam/sdk/services/motion/client.hpp       |  1 -
 src/viam/sdk/tests/mocks/mlmodel_mocks.hpp    |  1 +
 29 files changed, 42 insertions(+), 48 deletions(-)

diff --git a/src/viam/examples/modules/complex/gizmo/api.cpp b/src/viam/examples/modules/complex/gizmo/api.cpp
index 19b38fd95..9dd1a59f6 100644
--- a/src/viam/examples/modules/complex/gizmo/api.cpp
+++ b/src/viam/examples/modules/complex/gizmo/api.cpp
@@ -296,8 +296,6 @@ std::vector<bool> GizmoClient::do_one_bidi_stream(std::vector<std::string> arg1)
     return rets;
 }
 
-void GizmoClient::stop(const AttributeMap& extra) {}
-
 std::string GizmoClient::do_two(bool arg1) {
     DoTwoRequest request;
     DoTwoResponse response;
diff --git a/src/viam/examples/modules/complex/gizmo/api.hpp b/src/viam/examples/modules/complex/gizmo/api.hpp
index c9078d7fe..dc44ac730 100644
--- a/src/viam/examples/modules/complex/gizmo/api.hpp
+++ b/src/viam/examples/modules/complex/gizmo/api.hpp
@@ -54,7 +54,6 @@ class GizmoClient : public Gizmo {
     std::vector<bool> do_one_server_stream(std::string arg1) override;
     std::vector<bool> do_one_bidi_stream(std::vector<std::string> arg1) override;
     std::string do_two(bool arg1) override;
-    void stop(const AttributeMap& extra) override;
 
    private:
     std::unique_ptr<GizmoService::StubInterface> stub_;
diff --git a/src/viam/examples/modules/complex/summation/api.cpp b/src/viam/examples/modules/complex/summation/api.cpp
index 4f6862dbf..469fb9775 100644
--- a/src/viam/examples/modules/complex/summation/api.cpp
+++ b/src/viam/examples/modules/complex/summation/api.cpp
@@ -111,5 +111,3 @@ double SummationClient::sum(std::vector<double> numbers) {
 
     return response.sum();
 }
-
-void SummationClient::stop(const AttributeMap& extra) {}
diff --git a/src/viam/examples/modules/complex/summation/api.hpp b/src/viam/examples/modules/complex/summation/api.hpp
index 302ed5312..30d9a6c4d 100644
--- a/src/viam/examples/modules/complex/summation/api.hpp
+++ b/src/viam/examples/modules/complex/summation/api.hpp
@@ -49,7 +49,6 @@ class SummationClient : public Summation {
     SummationClient(std::string name, std::shared_ptr<grpc::Channel> channel);
 
     double sum(std::vector<double> numbers) override;
-    void stop(const AttributeMap& extra) override;
 
    private:
     std::unique_ptr<SummationService::StubInterface> stub_;
diff --git a/src/viam/examples/modules/tflite/main.cpp b/src/viam/examples/modules/tflite/main.cpp
index c7b4beea1..8f73c8fe6 100644
--- a/src/viam/examples/modules/tflite/main.cpp
+++ b/src/viam/examples/modules/tflite/main.cpp
@@ -73,7 +73,7 @@ class MLModelServiceTFLite : public vsdk::MLModelService {
         // drain.
     }
 
-    void stop(const vsdk::AttributeMap& extra) noexcept final {
+    void stop(const vsdk::AttributeMap& extra) noexcept final override {
         return stop();
     }
 
diff --git a/src/viam/sdk/components/base/base.hpp b/src/viam/sdk/components/base/base.hpp
index 71fb4230a..be1369bf4 100644
--- a/src/viam/sdk/components/base/base.hpp
+++ b/src/viam/sdk/components/base/base.hpp
@@ -37,7 +37,7 @@ class BaseRegistration : public ResourceRegistration {
 ///
 /// This acts as an abstract parent class to be inherited from by any drivers representing
 /// specific base implementations. This class cannot be used on its own.
-class Base : public Component {
+class Base : public Component, public Stoppable {
    public:
     /// @struct properties
     /// @brief Information about the physical base
diff --git a/src/viam/sdk/components/board/client.cpp b/src/viam/sdk/components/board/client.cpp
index 9ef99f7dd..e6d16cc1a 100644
--- a/src/viam/sdk/components/board/client.cpp
+++ b/src/viam/sdk/components/board/client.cpp
@@ -56,8 +56,6 @@ void BoardClient::set_gpio(const std::string& pin, bool high, const AttributeMap
     }
 }
 
-void BoardClient::stop(const AttributeMap& extra) {}
-
 bool BoardClient::get_gpio(const std::string& pin, const AttributeMap& extra) {
     viam::component::board::v1::GetGPIORequest request;
     viam::component::board::v1::GetGPIOResponse response;
diff --git a/src/viam/sdk/components/board/client.hpp b/src/viam/sdk/components/board/client.hpp
index cf35dcaa4..91526bc3b 100644
--- a/src/viam/sdk/components/board/client.hpp
+++ b/src/viam/sdk/components/board/client.hpp
@@ -21,7 +21,6 @@ namespace sdk {
 class BoardClient : public Board {
    public:
     BoardClient(std::string name, std::shared_ptr<grpc::Channel> channel);
-    void stop(const AttributeMap& extra) override;
     AttributeMap do_command(const AttributeMap& command) override;
     status get_status(const AttributeMap& extra) override;
     void set_gpio(const std::string& pin, bool high, const AttributeMap& extra) override;
diff --git a/src/viam/sdk/components/camera/client.cpp b/src/viam/sdk/components/camera/client.cpp
index 1e1e7e318..c9ffd2fc3 100644
--- a/src/viam/sdk/components/camera/client.cpp
+++ b/src/viam/sdk/components/camera/client.cpp
@@ -102,7 +102,5 @@ Camera::properties CameraClient::get_properties() {
     return from_proto(resp);
 };
 
-void CameraClient::stop(const AttributeMap& extra) {}
-
 }  // namespace sdk
 }  // namespace viam
diff --git a/src/viam/sdk/components/camera/client.hpp b/src/viam/sdk/components/camera/client.hpp
index c64e9adfc..204fc4b70 100644
--- a/src/viam/sdk/components/camera/client.hpp
+++ b/src/viam/sdk/components/camera/client.hpp
@@ -21,7 +21,6 @@ namespace sdk {
 /// @ingroup Camera
 class CameraClient : public Camera {
    public:
-    void stop(const AttributeMap& extra) override;
     CameraClient(std::string name, std::shared_ptr<grpc::Channel> channel);
     AttributeMap do_command(AttributeMap command) override;
     raw_image get_image(std::string mime_type, const AttributeMap& extra) override;
diff --git a/src/viam/sdk/components/encoder/client.cpp b/src/viam/sdk/components/encoder/client.cpp
index aa6f6a045..36ceaa026 100644
--- a/src/viam/sdk/components/encoder/client.cpp
+++ b/src/viam/sdk/components/encoder/client.cpp
@@ -105,7 +105,5 @@ AttributeMap EncoderClient::do_command(AttributeMap command) {
     return struct_to_map(response.result());
 }
 
-void EncoderClient::stop(const AttributeMap& extra) {}
-
 }  // namespace sdk
 }  // namespace viam
diff --git a/src/viam/sdk/components/encoder/client.hpp b/src/viam/sdk/components/encoder/client.hpp
index 18cf598aa..2374b3c39 100644
--- a/src/viam/sdk/components/encoder/client.hpp
+++ b/src/viam/sdk/components/encoder/client.hpp
@@ -21,7 +21,6 @@ namespace sdk {
 class EncoderClient : public Encoder {
    public:
     EncoderClient(std::string name, std::shared_ptr<grpc::Channel> channel);
-    void stop(const AttributeMap& extra) override;
     position get_position(const AttributeMap& extra, position_type position_type) override;
     void reset_position(const AttributeMap& extra) override;
     properties get_properties(const AttributeMap& extra) override;
diff --git a/src/viam/sdk/components/generic/client.cpp b/src/viam/sdk/components/generic/client.cpp
index 125b0688a..79779e1c3 100644
--- a/src/viam/sdk/components/generic/client.cpp
+++ b/src/viam/sdk/components/generic/client.cpp
@@ -31,8 +31,6 @@ AttributeMap GenericClient::do_command(AttributeMap command) {
     return struct_to_map(resp.result());
 }
 
-void GenericClient::stop(const AttributeMap& extra) {}
-
 std::vector<GeometryConfig> GenericClient::get_geometries() {
     viam::common::v1::GetGeometriesRequest req;
     viam::common::v1::GetGeometriesResponse resp;
diff --git a/src/viam/sdk/components/generic/client.hpp b/src/viam/sdk/components/generic/client.hpp
index db6eda12c..2cd13fda5 100644
--- a/src/viam/sdk/components/generic/client.hpp
+++ b/src/viam/sdk/components/generic/client.hpp
@@ -18,7 +18,6 @@ namespace sdk {
 /// @ingroup Generic
 class GenericClient : public Generic {
    public:
-    void stop(const AttributeMap& extra) override;
     GenericClient(std::string name, std::shared_ptr<grpc::Channel> channel);
     AttributeMap do_command(AttributeMap command) override;
     std::vector<GeometryConfig> get_geometries() override;
diff --git a/src/viam/sdk/components/motor/motor.hpp b/src/viam/sdk/components/motor/motor.hpp
index b378c4407..9d4db5a70 100644
--- a/src/viam/sdk/components/motor/motor.hpp
+++ b/src/viam/sdk/components/motor/motor.hpp
@@ -36,7 +36,7 @@ class MotorRegistration : public ResourceRegistration {
 ///
 /// This acts as an abstract base class to be inherited from by any drivers representing
 /// specific motor implementations. This class cannot be used on its own.
-class Motor : public Component {
+class Motor : public Component, public Stoppable {
    public:
     /// @struct position
     /// @brief Current position of the motor relative to its home
diff --git a/src/viam/sdk/components/movement_sensor/client.cpp b/src/viam/sdk/components/movement_sensor/client.cpp
index 329d233f2..09ac0097e 100644
--- a/src/viam/sdk/components/movement_sensor/client.cpp
+++ b/src/viam/sdk/components/movement_sensor/client.cpp
@@ -94,7 +94,5 @@ std::vector<GeometryConfig> MovementSensorClient::get_geometries(const Attribute
         .invoke([](auto& response) { return GeometryConfig::from_proto(response); });
 }
 
-void MovementSensorClient::stop(const AttributeMap& extra) {}
-
 }  // namespace sdk
 }  // namespace viam
diff --git a/src/viam/sdk/components/movement_sensor/client.hpp b/src/viam/sdk/components/movement_sensor/client.hpp
index 259aff315..06fdeab58 100644
--- a/src/viam/sdk/components/movement_sensor/client.hpp
+++ b/src/viam/sdk/components/movement_sensor/client.hpp
@@ -22,7 +22,6 @@ namespace sdk {
 class MovementSensorClient : public MovementSensor {
    public:
     MovementSensorClient(std::string name, std::shared_ptr<grpc::Channel> channel);
-    void stop(const AttributeMap& extra) override;
     Vector3 get_linear_velocity(const AttributeMap& extra) override;
     Vector3 get_angular_velocity(const AttributeMap& extra) override;
     compassheading get_compass_heading(const AttributeMap& extra) override;
diff --git a/src/viam/sdk/components/power_sensor/client.cpp b/src/viam/sdk/components/power_sensor/client.cpp
index 482c5c6f2..4576cbeee 100644
--- a/src/viam/sdk/components/power_sensor/client.cpp
+++ b/src/viam/sdk/components/power_sensor/client.cpp
@@ -62,7 +62,5 @@ AttributeMap PowerSensorClient::do_command(const AttributeMap& command) {
         .invoke([](auto& response) { return struct_to_map(response.result()); });
 }
 
-void PowerSensorClient::stop(const AttributeMap& extra) {}
-
 }  // namespace sdk
 }  // namespace viam
diff --git a/src/viam/sdk/components/power_sensor/client.hpp b/src/viam/sdk/components/power_sensor/client.hpp
index 8103df0ed..3963fc01d 100644
--- a/src/viam/sdk/components/power_sensor/client.hpp
+++ b/src/viam/sdk/components/power_sensor/client.hpp
@@ -20,7 +20,6 @@ namespace sdk {
 /// @ingroup PowerSensor
 class PowerSensorClient : public PowerSensor {
    public:
-    void stop(const AttributeMap& extra) override;
     PowerSensorClient(std::string name, std::shared_ptr<grpc::Channel> channel);
     voltage get_voltage(const AttributeMap& extra) override;
     current get_current(const AttributeMap& extra) override;
diff --git a/src/viam/sdk/components/sensor/client.cpp b/src/viam/sdk/components/sensor/client.cpp
index f4e47d405..2054c4868 100644
--- a/src/viam/sdk/components/sensor/client.cpp
+++ b/src/viam/sdk/components/sensor/client.cpp
@@ -49,7 +49,5 @@ std::vector<GeometryConfig> SensorClient::get_geometries(const AttributeMap& ext
         .invoke([](auto& response) { return GeometryConfig::from_proto(response); });
 }
 
-void SensorClient::stop(const AttributeMap& extra) {}
-
 }  // namespace sdk
 }  // namespace viam
diff --git a/src/viam/sdk/components/sensor/client.hpp b/src/viam/sdk/components/sensor/client.hpp
index f4639da22..5870274a2 100644
--- a/src/viam/sdk/components/sensor/client.hpp
+++ b/src/viam/sdk/components/sensor/client.hpp
@@ -19,7 +19,6 @@ namespace sdk {
 /// @ingroup Sensor
 class SensorClient : public Sensor {
    public:
-    void stop(const AttributeMap& extra) override;
     SensorClient(std::string name, std::shared_ptr<grpc::Channel> channel);
     AttributeMap get_readings(const AttributeMap& extra) override;
     AttributeMap do_command(const AttributeMap& command) override;
diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp
index ca26a6b49..7b26de2b8 100644
--- a/src/viam/sdk/module/service.cpp
+++ b/src/viam/sdk/module/service.cpp
@@ -133,7 +133,7 @@ ::grpc::Status ModuleService_::ReconfigureResource(
 
     // if the type isn't reconfigurable by default, replace it
     try {
-        res->stop();
+        stop_resource_if_stoppable(res);
     } catch (const std::exception& err) {
         BOOST_LOG_TRIVIAL(error) << "unable to stop resource: " << err.what();
     }
@@ -194,7 +194,7 @@ ::grpc::Status ModuleService_::RemoveResource(
     }
 
     try {
-        res->stop();
+        stop_resource_if_stoppable(res);
     } catch (const std::exception& err) {
         BOOST_LOG_TRIVIAL(error) << "unable to stop resource: " << err.what();
     }
diff --git a/src/viam/sdk/resource/resource.cpp b/src/viam/sdk/resource/resource.cpp
index 59f2075d9..ec3342c84 100644
--- a/src/viam/sdk/resource/resource.cpp
+++ b/src/viam/sdk/resource/resource.cpp
@@ -38,5 +38,12 @@ ResourceName Resource::get_resource_name(std::string name) {
     return r;
 }
 
+void stop_resource_if_stoppable(std::shared_ptr<Resource> resource) {
+    auto stoppable_res = std::dynamic_pointer_cast<Stoppable>(resource);
+    if (stoppable_res != NULL) {
+        stoppable_res->stop();
+    }
+}
+
 }  // namespace sdk
 }  // namespace viam
diff --git a/src/viam/sdk/resource/resource.hpp b/src/viam/sdk/resource/resource.hpp
index a190ecfc1..592019c56 100644
--- a/src/viam/sdk/resource/resource.hpp
+++ b/src/viam/sdk/resource/resource.hpp
@@ -27,15 +27,6 @@ class Resource {
     /// @brief Returns a `ResourceName` for a particular resource name.
     virtual viam::common::v1::ResourceName get_resource_name(std::string name);
 
-    /// @brief Stops a resource from running.
-    /// @param extra Extra arguments to pass to the resource's `stop` method.
-    virtual void stop(const AttributeMap& extra) = 0;
-
-    /// @brief Stops a resource from running.
-    inline void stop() {
-        return stop({});
-    }
-
     /// @brief Reconfigures a resource.
     /// @param deps Dependencies of the resource.
     /// @param cfg The resource's config.
@@ -51,5 +42,21 @@ class Resource {
     std::string name_;
 };
 
+class Stoppable {
+   public:
+    /// @brief Stops a resource from running.
+    /// @param extra Extra arguments to pass to the resource's `stop` method.
+    virtual void stop(const AttributeMap& extra) = 0;
+
+    /// @brief Stops a resource from running.
+    inline void stop() {
+        return stop({});
+    }
+};
+
+/// @brief Stops a Resource if it is Stoppable.
+/// @param resource The Resource to stop.
+void stop_resource_if_stoppable(std::shared_ptr<Resource> resource);
+
 }  // namespace sdk
 }  // namespace viam
diff --git a/src/viam/sdk/robot/service.cpp b/src/viam/sdk/robot/service.cpp
index e54da87cd..be614a125 100644
--- a/src/viam/sdk/robot/service.cpp
+++ b/src/viam/sdk/robot/service.cpp
@@ -164,11 +164,11 @@ ::grpc::Status RobotService_::StopAll(::grpc::ServerContext* context,
         const std::string rn_ = rn.SerializeAsString();
         if (extra.find(rn_) != extra.end()) {
             try {
-                resource->stop(extra.at(rn_));
+                stop_resource_if_stoppable(resource);
             } catch (const std::runtime_error& err) {
                 try {
                     status_message = err.what();
-                    resource->stop();
+                    stop_resource_if_stoppable(resource);
                 } catch (std::runtime_error& err) {
                     status_message = err.what();
                     status = grpc::UNKNOWN;
@@ -179,7 +179,7 @@ ::grpc::Status RobotService_::StopAll(::grpc::ServerContext* context,
             }
         } else {
             try {
-                resource->stop();
+                stop_resource_if_stoppable(resource);
             } catch (std::runtime_error& err) {
                 status_message = err.what();
                 status = grpc::UNKNOWN;
diff --git a/src/viam/sdk/services/mlmodel/mlmodel.hpp b/src/viam/sdk/services/mlmodel/mlmodel.hpp
index e58fdf750..52b5c4e9f 100644
--- a/src/viam/sdk/services/mlmodel/mlmodel.hpp
+++ b/src/viam/sdk/services/mlmodel/mlmodel.hpp
@@ -47,7 +47,7 @@ class MLModelServiceRegistration : public ResourceRegistration {
 /// This class acts as an abstract base class to be used by any driver
 /// implementing an MLModel. It is also used as the base class for the
 /// gRPC client for communicating with a remote MLModel instance.
-class MLModelService : public Service {
+class MLModelService : public Service, public Stoppable {
    private:
     template <typename T>
     struct make_tensor_view_ {
@@ -180,6 +180,15 @@ class MLModelService : public Service {
     /// @param `extra`: Any additional arguments to the method.
     virtual struct metadata metadata(const AttributeMap& extra) = 0;
 
+    /// @brief Stops a resource from running.
+    inline void stop() {
+        return stop({});
+    }
+
+    /// @brief Stops a resource from running.
+    /// @param extra Extra arguments to pass to the resource's `stop` method.
+    void stop(const AttributeMap& extra) override = 0;
+
    protected:
     explicit MLModelService(std::string name);
 };
diff --git a/src/viam/sdk/services/motion/client.cpp b/src/viam/sdk/services/motion/client.cpp
index e49298428..3d183cc9f 100644
--- a/src/viam/sdk/services/motion/client.cpp
+++ b/src/viam/sdk/services/motion/client.cpp
@@ -151,7 +151,5 @@ AttributeMap MotionClient::do_command(const AttributeMap& command) {
     return struct_to_map(response.result());
 }
 
-void MotionClient::stop(const AttributeMap& extra) {}
-
 }  // namespace sdk
 }  // namespace viam
diff --git a/src/viam/sdk/services/motion/client.hpp b/src/viam/sdk/services/motion/client.hpp
index a4622cd4f..949c18cee 100644
--- a/src/viam/sdk/services/motion/client.hpp
+++ b/src/viam/sdk/services/motion/client.hpp
@@ -16,7 +16,6 @@ namespace sdk {
 class MotionClient : public Motion {
    public:
     MotionClient(std::string name, std::shared_ptr<grpc::Channel> channel);
-    void stop(const AttributeMap& extra) override;
     bool move(const pose_in_frame& destination,
               const Name& component_name,
               std::shared_ptr<WorldState> world_state,
diff --git a/src/viam/sdk/tests/mocks/mlmodel_mocks.hpp b/src/viam/sdk/tests/mocks/mlmodel_mocks.hpp
index 44fccf53d..a6421cbb7 100644
--- a/src/viam/sdk/tests/mocks/mlmodel_mocks.hpp
+++ b/src/viam/sdk/tests/mocks/mlmodel_mocks.hpp
@@ -32,6 +32,7 @@ class MockMLModelService : public sdk::MLModelService {
     MockMLModelService& set_infer_handler(infer_handler handler);
     std::shared_ptr<named_tensor_views> infer(const named_tensor_views& inputs,
                                               const sdk::AttributeMap& extra) override;
+    void stop(const sdk::AttributeMap& extra) override{};
 
     MockMLModelService& set_metadata(struct metadata metadata);
     struct metadata metadata(const sdk::AttributeMap& extra) override;