From b29142440a48c5794da5df00f8df0901fb35b3ab Mon Sep 17 00:00:00 2001 From: CursedRock17 Date: Tue, 1 Aug 2023 18:38:04 -0400 Subject: [PATCH 1/2] Adding Missing Group Exceptions Signed-off-by: CursedRock17 --- rclcpp/include/rclcpp/exceptions/exceptions.hpp | 8 ++++++++ rclcpp/src/rclcpp/node_interfaces/node_services.cpp | 6 ++---- rclcpp/src/rclcpp/node_interfaces/node_timers.cpp | 3 +-- rclcpp/src/rclcpp/node_interfaces/node_topics.cpp | 5 ++--- rclcpp/src/rclcpp/node_interfaces/node_waitables.cpp | 3 +-- rclcpp/test/rclcpp/node_interfaces/test_node_services.cpp | 4 ++-- rclcpp/test/rclcpp/node_interfaces/test_node_timers.cpp | 2 +- .../test/rclcpp/node_interfaces/test_node_waitables.cpp | 2 +- 8 files changed, 18 insertions(+), 15 deletions(-) diff --git a/rclcpp/include/rclcpp/exceptions/exceptions.hpp b/rclcpp/include/rclcpp/exceptions/exceptions.hpp index ecb2a09905..d2c745d69f 100644 --- a/rclcpp/include/rclcpp/exceptions/exceptions.hpp +++ b/rclcpp/include/rclcpp/exceptions/exceptions.hpp @@ -222,6 +222,14 @@ class EventNotRegisteredError : public std::runtime_error : std::runtime_error("event already registered") {} }; +/// Thrown when a group is missing from the node, when it wants to utilize the group. +class MissingGroupNodeException : public std::runtime_error +{ +public: + explicit MissingGroupNodeException(const std::string & obj_type) + : std::runtime_error("cannot create: " + obj_type + " , callback group not in node") {} +}; + /// Thrown if passed parameters are inconsistent or invalid class InvalidParametersException : public std::runtime_error { diff --git a/rclcpp/src/rclcpp/node_interfaces/node_services.cpp b/rclcpp/src/rclcpp/node_interfaces/node_services.cpp index e9c4a5266e..fdd4e83780 100644 --- a/rclcpp/src/rclcpp/node_interfaces/node_services.cpp +++ b/rclcpp/src/rclcpp/node_interfaces/node_services.cpp @@ -32,8 +32,7 @@ NodeServices::add_service( { if (group) { if (!node_base_->callback_group_in_node(group)) { - // TODO(jacquelinekay): use custom exception - throw std::runtime_error("Cannot create service, group not in node."); + throw rclcpp::exceptions::MissingGroupNodeException("service"); } } else { group = node_base_->get_default_callback_group(); @@ -58,8 +57,7 @@ NodeServices::add_client( { if (group) { if (!node_base_->callback_group_in_node(group)) { - // TODO(jacquelinekay): use custom exception - throw std::runtime_error("Cannot create client, group not in node."); + throw rclcpp::exceptions::MissingGroupNodeException("client"); } } else { group = node_base_->get_default_callback_group(); diff --git a/rclcpp/src/rclcpp/node_interfaces/node_timers.cpp b/rclcpp/src/rclcpp/node_interfaces/node_timers.cpp index e72ddf91d3..29d3125e1f 100644 --- a/rclcpp/src/rclcpp/node_interfaces/node_timers.cpp +++ b/rclcpp/src/rclcpp/node_interfaces/node_timers.cpp @@ -34,8 +34,7 @@ NodeTimers::add_timer( { if (callback_group) { if (!node_base_->callback_group_in_node(callback_group)) { - // TODO(jacquelinekay): use custom exception - throw std::runtime_error("Cannot create timer, group not in node."); + throw rclcpp::exceptions::MissingGroupNodeException("timer"); } } else { callback_group = node_base_->get_default_callback_group(); diff --git a/rclcpp/src/rclcpp/node_interfaces/node_topics.cpp b/rclcpp/src/rclcpp/node_interfaces/node_topics.cpp index 659129d62c..ce71036b93 100644 --- a/rclcpp/src/rclcpp/node_interfaces/node_topics.cpp +++ b/rclcpp/src/rclcpp/node_interfaces/node_topics.cpp @@ -58,7 +58,7 @@ NodeTopics::add_publisher( // Assign to a group. if (callback_group) { if (!node_base_->callback_group_in_node(callback_group)) { - throw std::runtime_error("Cannot create publisher, callback group not in node."); + throw rclcpp::exceptions::MissingGroupNodeException("publisher"); } } else { callback_group = node_base_->get_default_callback_group(); @@ -97,8 +97,7 @@ NodeTopics::add_subscription( // Assign to a group. if (callback_group) { if (!node_base_->callback_group_in_node(callback_group)) { - // TODO(jacquelinekay): use custom exception - throw std::runtime_error("Cannot create subscription, callback group not in node."); + throw rclcpp::exceptions::MissingGroupNodeException("subscription"); } } else { callback_group = node_base_->get_default_callback_group(); diff --git a/rclcpp/src/rclcpp/node_interfaces/node_waitables.cpp b/rclcpp/src/rclcpp/node_interfaces/node_waitables.cpp index 02a9de82b0..96eb8df9cf 100644 --- a/rclcpp/src/rclcpp/node_interfaces/node_waitables.cpp +++ b/rclcpp/src/rclcpp/node_interfaces/node_waitables.cpp @@ -32,8 +32,7 @@ NodeWaitables::add_waitable( { if (group) { if (!node_base_->callback_group_in_node(group)) { - // TODO(jacobperron): use custom exception - throw std::runtime_error("Cannot create waitable, group not in node."); + throw rclcpp::exceptions::MissingGroupNodeException("waitable"); } } else { group = node_base_->get_default_callback_group(); diff --git a/rclcpp/test/rclcpp/node_interfaces/test_node_services.cpp b/rclcpp/test/rclcpp/node_interfaces/test_node_services.cpp index ea55a1aac2..7b00fea972 100644 --- a/rclcpp/test/rclcpp/node_interfaces/test_node_services.cpp +++ b/rclcpp/test/rclcpp/node_interfaces/test_node_services.cpp @@ -92,7 +92,7 @@ TEST_F(TestNodeService, add_service) different_node->create_callback_group(rclcpp::CallbackGroupType::MutuallyExclusive); RCLCPP_EXPECT_THROW_EQ( node_services->add_service(service, callback_group_in_different_node), - std::runtime_error("Cannot create service, group not in node.")); + rclcpp::exceptions::MissingGroupNodeException("service")); } TEST_F(TestNodeService, add_service_rcl_trigger_guard_condition_error) @@ -119,7 +119,7 @@ TEST_F(TestNodeService, add_client) different_node->create_callback_group(rclcpp::CallbackGroupType::MutuallyExclusive); RCLCPP_EXPECT_THROW_EQ( node_services->add_client(client, callback_group_in_different_node), - std::runtime_error("Cannot create client, group not in node.")); + rclcpp::exceptions::MissingGroupNodeException("client")); } TEST_F(TestNodeService, add_client_rcl_trigger_guard_condition_error) diff --git a/rclcpp/test/rclcpp/node_interfaces/test_node_timers.cpp b/rclcpp/test/rclcpp/node_interfaces/test_node_timers.cpp index d038a4b44d..dc92dee610 100644 --- a/rclcpp/test/rclcpp/node_interfaces/test_node_timers.cpp +++ b/rclcpp/test/rclcpp/node_interfaces/test_node_timers.cpp @@ -75,7 +75,7 @@ TEST_F(TestNodeTimers, add_timer) different_node->create_callback_group(rclcpp::CallbackGroupType::MutuallyExclusive); RCLCPP_EXPECT_THROW_EQ( node_timers->add_timer(timer, callback_group_in_different_node), - std::runtime_error("Cannot create timer, group not in node.")); + rclcpp::exceptions::MissingGroupNodeException("timer")); } TEST_F(TestNodeTimers, add_timer_rcl_trigger_guard_condition_error) diff --git a/rclcpp/test/rclcpp/node_interfaces/test_node_waitables.cpp b/rclcpp/test/rclcpp/node_interfaces/test_node_waitables.cpp index a1f35c0cdc..e559e57cd1 100644 --- a/rclcpp/test/rclcpp/node_interfaces/test_node_waitables.cpp +++ b/rclcpp/test/rclcpp/node_interfaces/test_node_waitables.cpp @@ -78,7 +78,7 @@ TEST_F(TestNodeWaitables, add_remove_waitable) node_waitables->add_waitable(waitable, callback_group1)); RCLCPP_EXPECT_THROW_EQ( node_waitables->add_waitable(waitable, callback_group2), - std::runtime_error("Cannot create waitable, group not in node.")); + rclcpp::exceptions::MissingGroupNodeException("waitable")); EXPECT_NO_THROW(node_waitables->remove_waitable(waitable, callback_group1)); EXPECT_NO_THROW(node_waitables->remove_waitable(waitable, callback_group2)); From 7eea142a3ad4302f0ebd81c64ad50eef87beff25 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Fri, 4 Aug 2023 10:14:50 -0400 Subject: [PATCH 2/2] Update rclcpp/include/rclcpp/exceptions/exceptions.hpp Signed-off-by: Chris Lalancette Co-authored-by: Tomoya Fujita --- rclcpp/include/rclcpp/exceptions/exceptions.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/include/rclcpp/exceptions/exceptions.hpp b/rclcpp/include/rclcpp/exceptions/exceptions.hpp index d2c745d69f..36ffc06c49 100644 --- a/rclcpp/include/rclcpp/exceptions/exceptions.hpp +++ b/rclcpp/include/rclcpp/exceptions/exceptions.hpp @@ -222,7 +222,7 @@ class EventNotRegisteredError : public std::runtime_error : std::runtime_error("event already registered") {} }; -/// Thrown when a group is missing from the node, when it wants to utilize the group. +/// Thrown when a callback group is missing from the node, when it wants to utilize the group. class MissingGroupNodeException : public std::runtime_error { public: