From a8cd93623964a089ee132d65e8b5eda84c15740d Mon Sep 17 00:00:00 2001 From: Claire Wang <22240514+claireyywang@users.noreply.github.com> Date: Mon, 29 Jun 2020 15:24:12 -0700 Subject: [PATCH] remove deprecated set_on_parameters_set_callback function (#1199) Signed-off-by: claireyywang <22240514+claireyywang@users.noreply.github.com> --- rclcpp/include/rclcpp/node.hpp | 20 -- .../node_interfaces/node_parameters.hpp | 5 - .../node_parameters_interface.hpp | 11 - rclcpp/src/rclcpp/node.cpp | 22 -- .../node_interfaces/node_parameters.cpp | 12 -- rclcpp/test/rclcpp/test_node.cpp | 200 ------------------ .../rclcpp_lifecycle/lifecycle_node.hpp | 11 - rclcpp_lifecycle/src/lifecycle_node.cpp | 22 -- 8 files changed, 303 deletions(-) diff --git a/rclcpp/include/rclcpp/node.hpp b/rclcpp/include/rclcpp/node.hpp index 08e42bd634..ba0c719383 100644 --- a/rclcpp/include/rclcpp/node.hpp +++ b/rclcpp/include/rclcpp/node.hpp @@ -842,26 +842,6 @@ class Node : public std::enable_shared_from_this void remove_on_set_parameters_callback(const OnSetParametersCallbackHandle * const handler); - /// Register a callback to be called anytime a parameter is about to be changed. - /** - * \deprecated Use add_on_set_parameters_callback instead. - * With this method, only one callback can be set at a time. The callback that was previously - * set by this method is returned or `nullptr` if no callback was previously set. - * - * The callbacks added with `add_on_set_parameters_callback` are stored in a different place. - * `remove_on_set_parameters_callback` can't be used with the callbacks registered with this - * method. For removing it, use `set_on_parameters_set_callback(nullptr)`. - * - * \param[in] callback The callback to be called when the value for a - * parameter is about to be set. - * \return The previous callback that was registered, if there was one, - * otherwise nullptr. - */ - [[deprecated("use add_on_set_parameters_callback(OnParametersSetCallbackType callback) instead")]] - RCLCPP_PUBLIC - OnParametersSetCallbackType - set_on_parameters_set_callback(rclcpp::Node::OnParametersSetCallbackType callback); - /// Get the fully-qualified names of all available nodes. /** * The fully-qualified name includes the local namespace and name of the node. diff --git a/rclcpp/include/rclcpp/node_interfaces/node_parameters.hpp b/rclcpp/include/rclcpp/node_interfaces/node_parameters.hpp index cf2676810a..4d70508eb7 100644 --- a/rclcpp/include/rclcpp/node_interfaces/node_parameters.hpp +++ b/rclcpp/include/rclcpp/node_interfaces/node_parameters.hpp @@ -167,11 +167,6 @@ class NodeParameters : public NodeParametersInterface void remove_on_set_parameters_callback(const OnSetParametersCallbackHandle * const handler) override; - [[deprecated("use add_on_set_parameters_callback(OnParametersSetCallbackType callback) instead")]] - RCLCPP_PUBLIC - OnParametersSetCallbackType - set_on_parameters_set_callback(OnParametersSetCallbackType callback) override; - RCLCPP_PUBLIC const std::map & get_parameter_overrides() const override; diff --git a/rclcpp/include/rclcpp/node_interfaces/node_parameters_interface.hpp b/rclcpp/include/rclcpp/node_interfaces/node_parameters_interface.hpp index c44763a09a..5b73998dbd 100644 --- a/rclcpp/include/rclcpp/node_interfaces/node_parameters_interface.hpp +++ b/rclcpp/include/rclcpp/node_interfaces/node_parameters_interface.hpp @@ -191,17 +191,6 @@ class NodeParametersInterface void remove_on_set_parameters_callback(const OnSetParametersCallbackHandle * const handler) = 0; - /// Register a callback for when parameters are being set, return an existing one. - /** - * \deprecated Use add_on_set_parameters_callback instead. - * \sa rclcpp::Node::set_on_parameters_set_callback - */ - [[deprecated("use add_on_set_parameters_callback(OnParametersSetCallbackType callback) instead")]] - RCLCPP_PUBLIC - virtual - OnParametersSetCallbackType - set_on_parameters_set_callback(OnParametersSetCallbackType callback) = 0; - /// Return the initial parameter values used by the NodeParameters to override default values. RCLCPP_PUBLIC virtual diff --git a/rclcpp/src/rclcpp/node.cpp b/rclcpp/src/rclcpp/node.cpp index 778d33ed01..b69d7c2baf 100644 --- a/rclcpp/src/rclcpp/node.cpp +++ b/rclcpp/src/rclcpp/node.cpp @@ -328,28 +328,6 @@ Node::remove_on_set_parameters_callback(const OnSetParametersCallbackHandle * co return node_parameters_->remove_on_set_parameters_callback(callback); } -// suppress deprecated function warning -#if !defined(_WIN32) -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wdeprecated-declarations" -#else // !defined(_WIN32) -# pragma warning(push) -# pragma warning(disable: 4996) -#endif - -rclcpp::Node::OnParametersSetCallbackType -Node::set_on_parameters_set_callback(rclcpp::Node::OnParametersSetCallbackType callback) -{ - return node_parameters_->set_on_parameters_set_callback(callback); -} - -// remove warning suppression -#if !defined(_WIN32) -# pragma GCC diagnostic pop -#else // !defined(_WIN32) -# pragma warning(pop) -#endif - std::vector Node::get_node_names() const { diff --git a/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp b/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp index 412fba46c5..90c851be90 100644 --- a/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp +++ b/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp @@ -874,18 +874,6 @@ NodeParameters::add_on_set_parameters_callback(OnParametersSetCallbackType callb return handle; } -NodeParameters::OnParametersSetCallbackType -NodeParameters::set_on_parameters_set_callback(OnParametersSetCallbackType callback) -{ - std::lock_guard lock(mutex_); - - ParameterMutationRecursionGuard guard(parameter_modification_enabled_); - - auto existing_callback = on_parameters_set_callback_; - on_parameters_set_callback_ = callback; - return existing_callback; -} - const std::map & NodeParameters::get_parameter_overrides() const { diff --git a/rclcpp/test/rclcpp/test_node.cpp b/rclcpp/test/rclcpp/test_node.cpp index f5cd2a2ae9..b3ff08d700 100644 --- a/rclcpp/test/rclcpp/test_node.cpp +++ b/rclcpp/test/rclcpp/test_node.cpp @@ -2329,206 +2329,6 @@ TEST_F(TestNode, get_parameter_types_undeclared_parameters_allowed) { } } -// suppress deprecated function test warnings -#if !defined(_WIN32) -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wdeprecated-declarations" -#else // !defined(_WIN32) -# pragma warning(push) -# pragma warning(disable: 4996) -#endif - -// test that it is possible to call get_parameter within the set_callback -TEST_F(TestNode, set_on_parameters_set_callback_get_parameter) { - auto node = std::make_shared("test_set_callback_get_parameter_node"_unq); - - int64_t intval = node->declare_parameter("intparam", 42); - EXPECT_EQ(intval, 42); - double floatval = node->declare_parameter("floatparam", 5.4); - EXPECT_EQ(floatval, 5.4); - - double floatout; - RCLCPP_SCOPE_EXIT({node->set_on_parameters_set_callback(nullptr);}); // always reset - auto on_set_parameters = - [&node, &floatout](const std::vector & parameters) { - rcl_interfaces::msg::SetParametersResult result; - result.successful = true; - if (parameters.size() != 1) { - result.successful = false; - } - - if (parameters[0].get_value() != 40) { - result.successful = false; - } - - rclcpp::Parameter floatparam = node->get_parameter("floatparam"); - if (floatparam.get_value() != 5.4) { - result.successful = false; - } - floatout = floatparam.get_value(); - - return result; - }; - - EXPECT_EQ(node->set_on_parameters_set_callback(on_set_parameters), nullptr); - ASSERT_NO_THROW(node->set_parameter({"intparam", 40})); - ASSERT_EQ(floatout, 5.4); -} - -// test that calling set_parameter inside of a set_callback throws an exception -TEST_F(TestNode, set_on_parameters_set_callback_set_parameter) { - auto node = std::make_shared("test_set_callback_set_parameter_node"_unq); - - int64_t intval = node->declare_parameter("intparam", 42); - EXPECT_EQ(intval, 42); - double floatval = node->declare_parameter("floatparam", 5.4); - EXPECT_EQ(floatval, 5.4); - - RCLCPP_SCOPE_EXIT({node->set_on_parameters_set_callback(nullptr);}); // always reset - auto on_set_parameters = - [&node](const std::vector & parameters) { - rcl_interfaces::msg::SetParametersResult result; - result.successful = true; - if (parameters.size() != 1) { - result.successful = false; - } - - if (parameters[0].get_value() != 40) { - result.successful = false; - } - - // This should throw an exception - node->set_parameter({"floatparam", 5.6}); - - return result; - }; - - EXPECT_EQ(node->set_on_parameters_set_callback(on_set_parameters), nullptr); - EXPECT_THROW( - { - node->set_parameter(rclcpp::Parameter("intparam", 40)); - }, rclcpp::exceptions::ParameterModifiedInCallbackException); -} - -// test that calling declare_parameter inside of a set_callback throws an exception -TEST_F(TestNode, set_on_parameters_set_callback_declare_parameter) { - auto node = std::make_shared("test_set_callback_declare_parameter_node"_unq); - - int64_t intval = node->declare_parameter("intparam", 42); - EXPECT_EQ(intval, 42); - double floatval = node->declare_parameter("floatparam", 5.4); - EXPECT_EQ(floatval, 5.4); - - RCLCPP_SCOPE_EXIT({node->set_on_parameters_set_callback(nullptr);}); // always reset - auto on_set_parameters = - [&node](const std::vector & parameters) { - rcl_interfaces::msg::SetParametersResult result; - result.successful = true; - if (parameters.size() != 1) { - result.successful = false; - } - - if (parameters[0].get_value() != 40) { - result.successful = false; - } - - // This should throw an exception - node->declare_parameter("floatparam2", 5.6); - - return result; - }; - - EXPECT_EQ(node->set_on_parameters_set_callback(on_set_parameters), nullptr); - EXPECT_THROW( - { - node->set_parameter(rclcpp::Parameter("intparam", 40)); - }, rclcpp::exceptions::ParameterModifiedInCallbackException); -} - -// test that calling undeclare_parameter inside a set_callback throws an exception -TEST_F(TestNode, set_on_parameters_set_callback_undeclare_parameter) { - auto node = std::make_shared("test_set_callback_undeclare_parameter_node"_unq); - - int64_t intval = node->declare_parameter("intparam", 42); - EXPECT_EQ(intval, 42); - double floatval = node->declare_parameter("floatparam", 5.4); - EXPECT_EQ(floatval, 5.4); - - RCLCPP_SCOPE_EXIT({node->set_on_parameters_set_callback(nullptr);}); // always reset - auto on_set_parameters = - [&node](const std::vector & parameters) { - rcl_interfaces::msg::SetParametersResult result; - result.successful = true; - if (parameters.size() != 1) { - result.successful = false; - } - - if (parameters[0].get_value() != 40) { - result.successful = false; - } - - // This should throw an exception - node->undeclare_parameter("floatparam"); - - return result; - }; - - EXPECT_EQ(node->set_on_parameters_set_callback(on_set_parameters), nullptr); - EXPECT_THROW( - { - node->set_parameter(rclcpp::Parameter("intparam", 40)); - }, rclcpp::exceptions::ParameterModifiedInCallbackException); -} - -// test that calling set_on_parameters_set_callback from a set_callback throws an exception -TEST_F(TestNode, set_on_parameters_set_callback_set_on_parameters_set_callback) { - auto node = std::make_shared("test_set_callback_set_callback_node"_unq); - - int64_t intval = node->declare_parameter("intparam", 42); - EXPECT_EQ(intval, 42); - double floatval = node->declare_parameter("floatparam", 5.4); - EXPECT_EQ(floatval, 5.4); - - RCLCPP_SCOPE_EXIT({node->set_on_parameters_set_callback(nullptr);}); // always reset - auto on_set_parameters = - [&node](const std::vector & parameters) { - rcl_interfaces::msg::SetParametersResult result; - result.successful = true; - if (parameters.size() != 1) { - result.successful = false; - } - - if (parameters[0].get_value() != 40) { - result.successful = false; - } - - auto bad_parameters = - [](const std::vector & parameters) { - (void)parameters; - rcl_interfaces::msg::SetParametersResult result; - return result; - }; - - // This should throw an exception - node->set_on_parameters_set_callback(bad_parameters); - - return result; - }; - - EXPECT_EQ(node->set_on_parameters_set_callback(on_set_parameters), nullptr); - EXPECT_THROW( - { - node->set_parameter(rclcpp::Parameter("intparam", 40)); - }, rclcpp::exceptions::ParameterModifiedInCallbackException); -} - -// remove warning suppression -#if !defined(_WIN32) -# pragma GCC diagnostic pop -#else // !defined(_WIN32) -# pragma warning(pop) -#endif - void expect_qos_profile_eq( const rmw_qos_profile_t & qos1, const rmw_qos_profile_t & qos2, bool is_publisher) { diff --git a/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp b/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp index bd2327fb97..9a7d6c977d 100644 --- a/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp +++ b/rclcpp_lifecycle/include/rclcpp_lifecycle/lifecycle_node.hpp @@ -469,17 +469,6 @@ class LifecycleNode : public node_interfaces::LifecycleNodeInterface, remove_on_set_parameters_callback( const rclcpp_lifecycle::LifecycleNode::OnSetParametersCallbackHandle * const handler); - /// Register a callback to be called anytime a parameter is about to be changed. - /** - * \deprecated Use add_on_set_parameters_callback instead. - * \sa rclcpp::Node::set_on_parameters_set_callback - */ - [[deprecated("use add_on_set_parameters_callback(OnParametersSetCallbackType callback) instead")]] - RCLCPP_LIFECYCLE_PUBLIC - rclcpp_lifecycle::LifecycleNode::OnParametersSetCallbackType - set_on_parameters_set_callback( - rclcpp_lifecycle::LifecycleNode::OnParametersSetCallbackType callback); - /// Return a vector of existing node names (string). /** * \sa rclcpp::Node::get_node_names diff --git a/rclcpp_lifecycle/src/lifecycle_node.cpp b/rclcpp_lifecycle/src/lifecycle_node.cpp index 1d7210546c..4da5289917 100644 --- a/rclcpp_lifecycle/src/lifecycle_node.cpp +++ b/rclcpp_lifecycle/src/lifecycle_node.cpp @@ -268,28 +268,6 @@ LifecycleNode::remove_on_set_parameters_callback( node_parameters_->remove_on_set_parameters_callback(callback); } -// suppress deprecated function warning -#if !defined(_WIN32) -# pragma GCC diagnostic push -# pragma GCC diagnostic ignored "-Wdeprecated-declarations" -#else // !defined(_WIN32) -# pragma warning(push) -# pragma warning(disable: 4996) -#endif - -rclcpp::Node::OnParametersSetCallbackType -LifecycleNode::set_on_parameters_set_callback(rclcpp::Node::OnParametersSetCallbackType callback) -{ - return node_parameters_->set_on_parameters_set_callback(callback); -} - -// remove warning suppression -#if !defined(_WIN32) -# pragma GCC diagnostic pop -#else // !defined(_WIN32) -# pragma warning(pop) -#endif - std::vector LifecycleNode::get_node_names() const {