adding dynamic model path reconfiguration

Edwardius · Edwardius · commit 9399e23cacfd · 2025-09-21T00:15:34.000Z
diff --git a/deep_core/CMakeLists.txt b/deep_core/CMakeLists.txt
@@ -75,6 +75,11 @@ if(BUILD_TESTING)
       ${DEEP_CORE_LIB}
   )
 
+  add_deep_test(test_dynamic_reconfiguration test/test_dynamic_reconfiguration.cpp
+    LIBRARIES
+      ${DEEP_CORE_LIB}
+  )
+
 endif()
 
 ament_export_targets(${PROJECT_NAME}Targets HAS_LIBRARY_TARGET)
diff --git a/deep_core/README.md b/deep_core/README.md
@@ -38,7 +38,8 @@ All nodes inherenting `deep_ros::DeepNodeBase` have the following settable param
 
 Required parameters:
 - `Backend.plugin`: Plugin name (e.g., "onnxruntime_cpu")
-- `model_path`: Path to model file
+- `model_path`: Path to model file (dynamically reconfigurable on runtime,
+you can switch model's while the node is running!)
 
 Optional parameters:
 - `Bond.enable`: Enable bond connections (default: false)
diff --git a/deep_core/include/deep_core/deep_node_base.hpp b/deep_core/include/deep_core/deep_node_base.hpp
@@ -20,6 +20,7 @@
 #include <vector>
 
 #include <bondcpp/bond.hpp>
+#include <lifecycle_msgs/msg/state.hpp>
 #include <pluginlib/class_list_macros.hpp>
 #include <pluginlib/class_loader.hpp>
 #include <rclcpp_lifecycle/lifecycle_node.hpp>
@@ -41,19 +42,12 @@ using CallbackReturn = rclcpp_lifecycle::node_interfaces::LifecycleNodeInterface
  * DeepNodeBase provides a lifecycle-managed ROS 2 node that can load and manage
  * deep learning backend plugins. It handles the plugin discovery, loading, model
  * management, and provides a simple interface for running inference.
- *
- * Users should inherit from this class and implement the lifecycle callback methods
- * to create custom inference nodes.
- *
- * Configuration parameters:
- * - Backend.plugin: Backend plugin name (e.g., "deep_ort_backend_plugin::OrtBackendPlugin")
- * - model_path: Path to the model file
  */
 class DeepNodeBase : public rclcpp_lifecycle::LifecycleNode
 {
 public:
   /**
-   * @brief Construct a new DeepNodeBase
+   * @brief Construct a new DeepNodeBase - initialize base parameters
    * @param node_name Name of the ROS 2 node
    * @param options ROS 2 node options
    */
@@ -172,15 +166,53 @@ class DeepNodeBase : public rclcpp_lifecycle::LifecycleNode
   std::shared_ptr<BackendMemoryAllocator> get_current_allocator() const;
 
 private:
-  // Final lifecycle callbacks - base handles backend, then calls user impl
+  /**
+   * @brief Configure lifecycle callback - retrieve parameter values,
+   * loads plugin and model, then calls user implementation
+   * @param state Current lifecycle state
+   * @return Callback return status
+   */
   CallbackReturn on_configure(const rclcpp_lifecycle::State & state) final;
+
+  /**
+   * @brief Activate lifecycle callback - starts bond if enabled, then calls user implementation
+   * @param state Current lifecycle state
+   * @return Callback return status
+   */
   CallbackReturn on_activate(const rclcpp_lifecycle::State & state) final;
+
+  /**
+   * @brief Deactivate lifecycle callback - calls user implementation
+   * @param state Current lifecycle state
+   * @return Callback return status
+   */
   CallbackReturn on_deactivate(const rclcpp_lifecycle::State & state) final;
+
+  /**
+   * @brief Cleanup lifecycle callback - unloads model/plugin, stops bond, then calls user implementation
+   * @param state Current lifecycle state
+   * @return Callback return status
+   */
   CallbackReturn on_cleanup(const rclcpp_lifecycle::State & state) final;
+
+  /**
+   * @brief Shutdown lifecycle callback - unloads model/plugin, stops bond, then calls user implementation
+   * @param state Current lifecycle state
+   * @return Callback return status
+   */
   CallbackReturn on_shutdown(const rclcpp_lifecycle::State & state) final;
 
-  // Plugin discovery and loading
+  /**
+   * @brief Discover available backend plugins using pluginlib
+   * @return Vector of plugin class names
+   */
   std::vector<std::string> discover_available_plugins();
+
+  /**
+   * @brief Load a specific backend plugin library
+   * @param plugin_name Name of the plugin class to load
+   * @return Unique pointer to the loaded plugin instance
+   */
   pluginlib::UniquePtr<DeepBackendPlugin> load_plugin_library(const std::string & plugin_name);
 
   // Plugin loader
@@ -201,6 +233,10 @@ class DeepNodeBase : public rclcpp_lifecycle::LifecycleNode
   // ROS parameters
   void declare_parameters();
   void setup_bond();
+  rcl_interfaces::msg::SetParametersResult on_parameter_change(const std::vector<rclcpp::Parameter> & parameters);
+
+  // Parameter callback for dynamic reconfiguration
+  rclcpp::node_interfaces::OnSetParametersCallbackHandle::SharedPtr parameter_callback_handle_;
 };
 
 }  // namespace deep_ros
diff --git a/deep_core/src/deep_node_base.cpp b/deep_core/src/deep_node_base.cpp
@@ -29,6 +29,10 @@ DeepNodeBase::DeepNodeBase(const std::string & node_name, const rclcpp::NodeOpti
   plugin_loader_ =
     std::make_unique<pluginlib::ClassLoader<DeepBackendPlugin>>("deep_core", "deep_ros::DeepBackendPlugin");
   declare_parameters();
+
+  // Set up parameter callback for dynamic reconfiguration
+  parameter_callback_handle_ =
+    add_on_set_parameters_callback(std::bind(&DeepNodeBase::on_parameter_change, this, std::placeholders::_1));
 }
 
 void DeepNodeBase::declare_parameters()
@@ -240,4 +244,54 @@ void DeepNodeBase::setup_bond()
   }
 }
 
+rcl_interfaces::msg::SetParametersResult DeepNodeBase::on_parameter_change(
+  const std::vector<rclcpp::Parameter> & parameters)
+{
+  rcl_interfaces::msg::SetParametersResult result;
+  result.successful = true;
+
+  for (const auto & param : parameters) {
+    if (param.get_name() == "model_path") {
+      // Only allow model changes when node is active for safety
+      if (get_current_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE) {
+        std::string new_model_path = param.as_string();
+
+        // Reject empty model paths entirely
+        if (new_model_path.empty()) {
+          RCLCPP_ERROR(get_logger(), "Cannot set empty model path");
+          result.successful = false;
+          result.reason = "Cannot set empty model path";
+        } else if (new_model_path != current_model_path_.string()) {
+          RCLCPP_INFO(
+            get_logger(),
+            "Dynamically changing model from '%s' to '%s'",
+            current_model_path_.c_str(),
+            new_model_path.c_str());
+
+          // Unload current model
+          unload_model();
+
+          // Load new model
+          if (!load_model(new_model_path)) {
+            RCLCPP_ERROR(get_logger(), "Failed to load new model: %s", new_model_path.c_str());
+            result.successful = false;
+            result.reason = "Failed to load new model: " + new_model_path;
+          } else {
+            RCLCPP_INFO(get_logger(), "Successfully loaded new model: %s", new_model_path.c_str());
+          }
+        }
+      } else {
+        RCLCPP_WARN(
+          get_logger(),
+          "Cannot change model_path when node is not active. Current state: %s",
+          get_current_state().label().c_str());
+        result.successful = false;
+        result.reason = "Node must be active to change model_path";
+      }
+    }
+  }
+
+  return result;
+}
+
 }  // namespace deep_ros
diff --git a/deep_core/src/tensor.cpp b/deep_core/src/tensor.cpp
@@ -82,6 +82,10 @@ TensorPtr::TensorPtr(
 , is_view_(false)
 , allocator_(allocator)
 {
+  if (shape_.empty()) {
+    throw std::invalid_argument("Tensor shape cannot be empty");
+  }
+
   calculate_strides();
 
   size_t total_elements = std::accumulate(shape_.begin(), shape_.end(), 1UL, std::multiplies<size_t>());
@@ -97,6 +101,10 @@ TensorPtr::TensorPtr(void * data, const std::vector<size_t> & shape, DataType dt
 , is_view_(false)
 , allocator_(nullptr)
 {
+  if (shape_.empty()) {
+    throw std::invalid_argument("Tensor shape cannot be empty");
+  }
+
   calculate_strides();
 
   size_t total_elements = std::accumulate(shape_.begin(), shape_.end(), 1UL, std::multiplies<size_t>());
diff --git a/deep_core/test/test_deep_core.cpp b/deep_core/test/test_deep_core.cpp
@@ -20,6 +20,7 @@
 #include <catch2/catch.hpp>
 #include <deep_core/deep_node_base.hpp>
 #include <deep_core/types/tensor.hpp>
+#include <deep_test/deep_test.hpp>
 
 namespace deep_ros
 {
@@ -114,15 +115,12 @@ TEST_CASE("Different data types have correct sizes", "[tensor]")
   REQUIRE(uint8_tensor.size() == 10);
 }
 
-TEST_CASE("Empty shape creates scalar tensor", "[tensor]")
+TEST_CASE("Empty shape throws exception", "[tensor]")
 {
   auto allocator = std::make_shared<MockMemoryAllocator>();
   std::vector<size_t> empty_shape;
 
-  TensorPtr tensor(empty_shape, DataType::FLOAT32, allocator);
-
-  REQUIRE(tensor.size() == 1);  // Scalar tensor
-  REQUIRE(tensor.shape().empty());
+  REQUIRE_THROWS_AS(TensorPtr(empty_shape, DataType::FLOAT32, allocator), std::invalid_argument);
 }
 
 TEST_CASE("Large shape allocation", "[tensor]")
@@ -263,6 +261,15 @@ class TestInferenceNode : public DeepNodeBase
   : DeepNodeBase("test_inference_node", options)
   {}
 
+  // Expose protected methods for testing
+  using DeepNodeBase::get_backend_name;
+  using DeepNodeBase::get_current_allocator;
+  using DeepNodeBase::is_model_loaded;
+  using DeepNodeBase::is_plugin_loaded;
+  using DeepNodeBase::load_model;
+  using DeepNodeBase::load_plugin;
+  using DeepNodeBase::run_inference;
+
   bool test_load_plugin(const std::string & plugin_name)
   {
     return load_plugin(plugin_name);
@@ -305,10 +312,73 @@ class TestInferenceNode : public DeepNodeBase
   }
 };
 
-TEST_CASE("DeepNodeBase creation", "[node]")
+TEST_CASE_METHOD(deep_ros::test::TestExecutorFixture, "DeepNodeBase lifecycle management", "[node]")
 {
-  // Skip ROS lifecycle node tests to avoid segfault
-  REQUIRE(true);
+  // Create a test node that inherits from DeepNodeBase
+  auto test_node = std::make_shared<TestInferenceNode>();
+  add_node(test_node);
+  start_spinning();
+
+  SECTION("Node creation and initial state")
+  {
+    REQUIRE(test_node->get_name() == std::string("test_inference_node"));
+    REQUIRE(test_node->get_current_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED);
+    REQUIRE(test_node->is_plugin_loaded() == false);
+    REQUIRE(test_node->is_model_loaded() == false);
+  }
+
+  SECTION("Lifecycle transitions work correctly")
+  {
+    // Configure
+    auto configure_result = test_node->configure();
+    REQUIRE(configure_result.id() == lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE);
+
+    // Activate
+    auto activate_result = test_node->activate();
+    REQUIRE(activate_result.id() == lifecycle_msgs::msg::State::PRIMARY_STATE_ACTIVE);
+
+    // Deactivate
+    auto deactivate_result = test_node->deactivate();
+    REQUIRE(deactivate_result.id() == lifecycle_msgs::msg::State::PRIMARY_STATE_INACTIVE);
+
+    // Cleanup
+    auto cleanup_result = test_node->cleanup();
+    REQUIRE(cleanup_result.id() == lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED);
+  }
+
+  SECTION("Plugin loading functionality")
+  {
+    // Configure first
+    test_node->configure();
+
+    // Try to load a non-existent plugin
+    bool load_result = test_node->test_load_plugin("nonexistent_plugin");
+    REQUIRE(load_result == false);
+    REQUIRE(test_node->is_plugin_loaded() == false);
+    REQUIRE(test_node->get_backend_name() == "none");
+    REQUIRE(test_node->get_current_allocator() == nullptr);
+  }
+
+  SECTION("Model loading without plugin fails")
+  {
+    test_node->configure();
+    test_node->activate();
+
+    // Try to load model without plugin
+    bool model_result = test_node->test_load_model("/fake/model.onnx");
+    REQUIRE(model_result == false);
+    REQUIRE(test_node->is_model_loaded() == false);
+  }
+
+  SECTION("Backend functionality with no plugin")
+  {
+    test_node->configure();
+    test_node->activate();
+
+    // Verify backend state when no plugin is loaded
+    REQUIRE(test_node->get_backend_name() == "none");
+    REQUIRE(test_node->get_current_allocator() == nullptr);
+  }
 }
 
 }  // namespace test
diff --git a/deep_core/test/test_dynamic_reconfiguration.cpp b/deep_core/test/test_dynamic_reconfiguration.cpp
diff --git a/deep_test/CMakeLists.txt b/deep_test/CMakeLists.txt
diff --git a/deep_test/cmake/add_deep_test.cmake b/deep_test/cmake/add_deep_test.cmake
