removed smell

Author: Edwardius

deep_core/README.md

@@ -21,7 +21,7 @@ Multi-dimensional tensor smart pointer supporting:
 Lifecycle node that handles:
 - Dynamic backend plugin loading via pluginlib
 - Model loading/unloading lifecycle
-- Optional bond connections for nav2 integration
+- Optional bond connections for nav2 integration and integration with other lifecycle managers
 - Parameter-driven configuration
 
 ### Plugin Interfaces

deep_core/include/deep_core/deep_node_base.hpp

@@ -56,7 +56,7 @@ class DeepNodeBase : public rclcpp_lifecycle::LifecycleNode
   /**
    * @brief Destructor
    */
-  virtual ~DeepNodeBase();
+  virtual ~DeepNodeBase() = default;
 
 protected:
   /**

deep_core/src/deep_node_base.cpp

@@ -29,18 +29,6 @@ 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));
-}
-
-DeepNodeBase::~DeepNodeBase()
-{
-  // Remove parameter callback to prevent callback invocation during destruction
-  if (parameter_callback_handle_) {
-    remove_on_set_parameters_callback(parameter_callback_handle_.get());
-  }
 }
 
 void DeepNodeBase::declare_parameters()
@@ -84,6 +72,9 @@ CallbackReturn DeepNodeBase::on_activate(const rclcpp_lifecycle::State & state)
 {
   RCLCPP_INFO(get_logger(), "Activating DeepNodeBase");
 
+  parameter_callback_handle_ =
+    add_on_set_parameters_callback(std::bind(&DeepNodeBase::on_parameter_change, this, std::placeholders::_1));
+
   // Start bond if enabled
   if (bond_enabled_ && bond_) {
     bond_->start();
@@ -96,6 +87,12 @@ CallbackReturn DeepNodeBase::on_activate(const rclcpp_lifecycle::State & state)
 CallbackReturn DeepNodeBase::on_deactivate(const rclcpp_lifecycle::State & state)
 {
   RCLCPP_INFO(get_logger(), "Deactivating DeepNodeBase");
+
+  if (parameter_callback_handle_) {
+    remove_on_set_parameters_callback(parameter_callback_handle_.get());
+    parameter_callback_handle_.reset();
+  }
+
   return on_deactivate_impl(state);
 }
 
@@ -118,6 +115,11 @@ CallbackReturn DeepNodeBase::on_shutdown(const rclcpp_lifecycle::State & state)
 {
   RCLCPP_INFO(get_logger(), "Shutting down DeepNodeBase");
 
+  if (parameter_callback_handle_) {
+    remove_on_set_parameters_callback(parameter_callback_handle_.get());
+    parameter_callback_handle_.reset();
+  }
+
   // Stop bond if active
   if (bond_) {
     bond_.reset();

deep_core/test/test_dynamic_parameters.cpp

@@ -88,16 +88,18 @@ TEST_CASE_METHOD(deep_ros::test::TestExecutorFixture, "Dynamic model reconfigura
     REQUIRE(test_node->get_current_state().id() == lifecycle_msgs::msg::State::PRIMARY_STATE_UNCONFIGURED);
   }
 
-  SECTION("Model parameter change rejected when not active")
+  SECTION("Model parameter change allowed when not active (no callback registered)")
   {
     // Try to set model_path while in unconfigured state
     auto parameters = std::vector<rclcpp::Parameter>{rclcpp::Parameter("model_path", "/some/model/path.onnx")};
 
     auto result = test_node->set_parameters(parameters);
 
-    // Should fail because node is not active
-    REQUIRE(result[0].successful == false);
-    REQUIRE(result[0].reason.find("Node must be active") != std::string::npos);
+    // Should succeed because callback is not registered until activation
+    REQUIRE(result[0].successful == true);
+
+    // Verify the parameter was set
+    REQUIRE(test_node->get_parameter("model_path").as_string() == "/some/model/path.onnx");
   }
 
   SECTION("Configuration fails with invalid plugin")