diff --git a/rclcpp_action/include/rclcpp_action/client_goal_handle.hpp b/rclcpp_action/include/rclcpp_action/client_goal_handle.hpp index 73987ec887..919a26f852 100644 --- a/rclcpp_action/include/rclcpp_action/client_goal_handle.hpp +++ b/rclcpp_action/include/rclcpp_action/client_goal_handle.hpp @@ -91,9 +91,21 @@ class ClientGoalHandle get_goal_stamp() const; /// Get the goal status code. + /** + * \deprecated Use get_status_code(), which returns a GoalStatusCode. + */ + [[deprecated("use get_status_code() instead")]] int8_t get_status(); + /// Get the goal status code as a GoalStatusCode. + /** + * \throws exceptions::InvalidGoalStatusError If the server sent a status that is not one of + * the action_msgs::msg::GoalStatus values. + */ + GoalStatusCode + get_status_code(); + /// Check if an action client has subscribed to feedback for the goal. bool is_feedback_aware(); diff --git a/rclcpp_action/include/rclcpp_action/client_goal_handle_impl.hpp b/rclcpp_action/include/rclcpp_action/client_goal_handle_impl.hpp index 58b1d7f248..06ba868fc2 100644 --- a/rclcpp_action/include/rclcpp_action/client_goal_handle_impl.hpp +++ b/rclcpp_action/include/rclcpp_action/client_goal_handle_impl.hpp @@ -103,6 +103,18 @@ ClientGoalHandle::get_status() return status_; } +template +GoalStatusCode +ClientGoalHandle::get_status_code() +{ + std::lock_guard guard(handle_mutex_); + const int8_t status = status_; + if (status < GoalStatus::STATUS_UNKNOWN || status > GoalStatus::STATUS_ABORTED) { + throw exceptions::InvalidGoalStatusError(status); + } + return static_cast(status); +} + template void ClientGoalHandle::set_status(int8_t status) diff --git a/rclcpp_action/include/rclcpp_action/exceptions.hpp b/rclcpp_action/include/rclcpp_action/exceptions.hpp index a1fcf50bff..20bc224d5a 100644 --- a/rclcpp_action/include/rclcpp_action/exceptions.hpp +++ b/rclcpp_action/include/rclcpp_action/exceptions.hpp @@ -15,6 +15,7 @@ #ifndef RCLCPP_ACTION__EXCEPTIONS_HPP_ #define RCLCPP_ACTION__EXCEPTIONS_HPP_ +#include #include #include @@ -41,6 +42,16 @@ class UnawareGoalHandleError : public std::runtime_error } }; +class InvalidGoalStatusError : public std::runtime_error +{ +public: + explicit InvalidGoalStatusError(int8_t status) + : std::runtime_error( + "Goal status " + std::to_string(status) + " is not an action_msgs/GoalStatus value.") + { + } +}; + } // namespace exceptions } // namespace rclcpp_action diff --git a/rclcpp_action/include/rclcpp_action/generic_client_goal_handle.hpp b/rclcpp_action/include/rclcpp_action/generic_client_goal_handle.hpp index 1d7fa34884..704937eea3 100644 --- a/rclcpp_action/include/rclcpp_action/generic_client_goal_handle.hpp +++ b/rclcpp_action/include/rclcpp_action/generic_client_goal_handle.hpp @@ -82,10 +82,23 @@ class GenericClientGoalHandle get_goal_stamp() const; /// Get the goal status code. + /** + * \deprecated Use get_status_code(), which returns a GoalStatusCode. + */ + [[deprecated("use get_status_code() instead")]] RCLCPP_ACTION_PUBLIC int8_t get_status(); + /// Get the goal status code as a GoalStatusCode. + /** + * \throws exceptions::InvalidGoalStatusError If the server sent a status that is not one of + * the action_msgs::msg::GoalStatus values. + */ + RCLCPP_ACTION_PUBLIC + GoalStatusCode + get_status_code(); + /// Check if an action client has subscribed to feedback for the goal. RCLCPP_ACTION_PUBLIC bool diff --git a/rclcpp_action/include/rclcpp_action/types.hpp b/rclcpp_action/include/rclcpp_action/types.hpp index 2033891aa9..f383673123 100644 --- a/rclcpp_action/include/rclcpp_action/types.hpp +++ b/rclcpp_action/include/rclcpp_action/types.hpp @@ -34,6 +34,18 @@ using GoalUUID = std::array; using GoalStatus = action_msgs::msg::GoalStatus; using GoalInfo = action_msgs::msg::GoalInfo; +/// Every status a goal can have, as defined in action_msgs::msg::GoalStatus. +enum class GoalStatusCode : int8_t +{ + UNKNOWN = GoalStatus::STATUS_UNKNOWN, + ACCEPTED = GoalStatus::STATUS_ACCEPTED, + EXECUTING = GoalStatus::STATUS_EXECUTING, + CANCELING = GoalStatus::STATUS_CANCELING, + SUCCEEDED = GoalStatus::STATUS_SUCCEEDED, + CANCELED = GoalStatus::STATUS_CANCELED, + ABORTED = GoalStatus::STATUS_ABORTED +}; + /// Convert a goal id to a human readable RFC-4122 compliant string. RCLCPP_ACTION_PUBLIC std::string diff --git a/rclcpp_action/src/generic_client_goal_handle.cpp b/rclcpp_action/src/generic_client_goal_handle.cpp index b0fad95903..78b2c74017 100644 --- a/rclcpp_action/src/generic_client_goal_handle.cpp +++ b/rclcpp_action/src/generic_client_goal_handle.cpp @@ -89,6 +89,17 @@ GenericClientGoalHandle::get_status() return status_; } +GoalStatusCode +GenericClientGoalHandle::get_status_code() +{ + std::lock_guard guard(handle_mutex_); + const int8_t status = status_; + if (status < GoalStatus::STATUS_UNKNOWN || status > GoalStatus::STATUS_ABORTED) { + throw exceptions::InvalidGoalStatusError(status); + } + return static_cast(status); +} + void GenericClientGoalHandle::set_status(int8_t status) { diff --git a/rclcpp_action/test/benchmark/benchmark_action_client.cpp b/rclcpp_action/test/benchmark/benchmark_action_client.cpp index 3587674cee..566a55caba 100644 --- a/rclcpp_action/test/benchmark/benchmark_action_client.cpp +++ b/rclcpp_action/test/benchmark/benchmark_action_client.cpp @@ -234,7 +234,7 @@ BENCHMARK_F(ActionClientPerformanceTest, async_send_goal_get_accepted_response)( } auto goal_handle = future_goal_handle.get(); - if (rclcpp_action::GoalStatus::STATUS_ACCEPTED != goal_handle->get_status()) { + if (rclcpp_action::GoalStatusCode::ACCEPTED != goal_handle->get_status_code()) { state.SkipWithError("Valid goal was not accepted"); return; } diff --git a/rclcpp_action/test/benchmark/benchmark_action_server.cpp b/rclcpp_action/test/benchmark/benchmark_action_server.cpp index 28017cf0e6..60d6f77411 100644 --- a/rclcpp_action/test/benchmark/benchmark_action_server.cpp +++ b/rclcpp_action/test/benchmark/benchmark_action_server.cpp @@ -157,7 +157,7 @@ BENCHMARK_F(ActionServerPerformanceTest, action_server_accept_goal)(benchmark::S rclcpp::spin_until_future_complete(node, client_goal_handle_future); auto goal_handle = client_goal_handle_future.get(); - if (rclcpp_action::GoalStatus::STATUS_ACCEPTED != goal_handle->get_status()) { + if (rclcpp_action::GoalStatusCode::ACCEPTED != goal_handle->get_status_code()) { state.SkipWithError("Valid goal was not accepted"); return; } @@ -226,7 +226,7 @@ BENCHMARK_F(ActionServerPerformanceTest, action_server_execute_goal)(benchmark:: rclcpp::spin_until_future_complete(node, client_goal_handle_future); auto goal_handle = client_goal_handle_future.get(); - if (rclcpp_action::GoalStatus::STATUS_ACCEPTED != goal_handle->get_status()) { + if (rclcpp_action::GoalStatusCode::ACCEPTED != goal_handle->get_status_code()) { state.SkipWithError("Valid goal was not accepted"); return; } @@ -272,7 +272,7 @@ BENCHMARK_F(ActionServerPerformanceTest, action_server_set_success)(benchmark::S rclcpp::spin_until_future_complete(node, client_goal_handle_future); auto goal_handle = client_goal_handle_future.get(); - if (rclcpp_action::GoalStatus::STATUS_ACCEPTED != goal_handle->get_status()) { + if (rclcpp_action::GoalStatusCode::ACCEPTED != goal_handle->get_status_code()) { state.SkipWithError("Valid goal was not accepted"); return; } @@ -317,7 +317,7 @@ BENCHMARK_F(ActionServerPerformanceTest, action_server_abort)(benchmark::State & rclcpp::spin_until_future_complete(node, client_goal_handle_future); auto goal_handle = client_goal_handle_future.get(); - if (rclcpp_action::GoalStatus::STATUS_ACCEPTED != goal_handle->get_status()) { + if (rclcpp_action::GoalStatusCode::ACCEPTED != goal_handle->get_status_code()) { state.SkipWithError("Valid goal was not accepted"); return; } diff --git a/rclcpp_action/test/test_client.cpp b/rclcpp_action/test/test_client.cpp index d11866251a..9f2f757c18 100644 --- a/rclcpp_action/test/test_client.cpp +++ b/rclcpp_action/test/test_client.cpp @@ -56,6 +56,8 @@ #include "rclcpp_action/qos.hpp" #include "rclcpp_action/types.hpp" +#include "rcpputils/compile_warnings.hpp" + #include "mocking_utils/patch.hpp" using namespace std::chrono_literals; @@ -596,7 +598,7 @@ TEST_F(TestClientAgainstServer, async_send_goal_no_callbacks) future_goal_handle = action_client->async_send_goal(good_goal); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); } @@ -628,13 +630,14 @@ TEST_F(TestClientAgainstServer, async_send_goal_no_callbacks_wait_for_result) auto future_goal_handle = action_client->async_send_goal(goal); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); auto future_result = action_client->async_get_result(goal_handle); EXPECT_TRUE(goal_handle->is_result_aware()); dual_spin_until_future_complete(future_result); auto wrapped_result = future_result.get(); + EXPECT_EQ(rclcpp_action::GoalStatusCode::SUCCEEDED, goal_handle->get_status_code()); ASSERT_EQ(6ul, wrapped_result.result->sequence.size()); EXPECT_EQ(0, wrapped_result.result->sequence[0]); EXPECT_EQ(1, wrapped_result.result->sequence[1]); @@ -652,17 +655,52 @@ TEST_F(TestClientAgainstServer, async_send_goal_no_callbacks_then_invalidate) dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); ASSERT_NE(nullptr, goal_handle); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); auto future_result = action_client->async_get_result(goal_handle); EXPECT_TRUE(goal_handle->is_result_aware()); action_client.reset(); // Ensure goal handle is invalidated once client goes out of scope - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_UNKNOWN, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::UNKNOWN, goal_handle->get_status_code()); using rclcpp_action::exceptions::UnawareGoalHandleError; EXPECT_THROW(future_result.get(), UnawareGoalHandleError); } +// get_status() is deprecated, but this test checks it still returns the raw value. +RCPPUTILS_DEPRECATION_WARNING_OFF_START +TEST_F(TestClientAgainstServer, get_status_code_out_of_range) +{ + auto action_client = rclcpp_action::create_client(client_node, action_name); + ASSERT_TRUE(action_client->wait_for_action_server(WAIT_FOR_SERVER_TIMEOUT)); + + ActionGoal goal; + goal.order = 5; + auto future_goal_handle = action_client->async_send_goal(goal); + dual_spin_until_future_complete(future_goal_handle); + auto goal_handle = future_goal_handle.get(); + ASSERT_NE(nullptr, goal_handle); + + // A server that does not follow the action protocol can send any int8_t as a status. + const int8_t out_of_range_status = 42; + ActionStatusMessage status_message; + rclcpp_action::GoalStatus goal_status; + goal_status.goal_info.goal_id.uuid = goal_handle->get_goal_id(); + goal_status.status = out_of_range_status; + status_message.status_list.push_back(goal_status); + status_publisher->publish(status_message); + + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (goal_handle->get_status() != out_of_range_status && + std::chrono::steady_clock::now() < deadline) + { + client_executor.spin_some(); + } + ASSERT_EQ(out_of_range_status, goal_handle->get_status()); + EXPECT_THROW( + goal_handle->get_status_code(), rclcpp_action::exceptions::InvalidGoalStatusError); +} +RCPPUTILS_DEPRECATION_WARNING_OFF_STOP + TEST_F(TestClientAgainstServer, async_send_goal_with_goal_response_callback_wait_for_result) { auto action_client = rclcpp_action::create_client(client_node, action_name); @@ -695,7 +733,7 @@ TEST_F(TestClientAgainstServer, async_send_goal_with_goal_response_callback_wait dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); EXPECT_TRUE(goal_response_received); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); auto future_result = action_client->async_get_result(goal_handle); @@ -725,7 +763,7 @@ TEST_F(TestClientAgainstServer, async_send_goal_with_feedback_callback_wait_for_ auto future_goal_handle = action_client->async_send_goal(goal, send_goal_ops); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_TRUE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); auto future_result = action_client->async_get_result(goal_handle); @@ -761,7 +799,7 @@ TEST_F(TestClientAgainstServer, async_send_goal_with_result_callback_wait_for_re auto future_goal_handle = action_client->async_send_goal(goal, send_goal_ops); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_TRUE(goal_handle->is_result_aware()); auto future_result = action_client->async_get_result(goal_handle); @@ -784,7 +822,7 @@ TEST_F(TestClientAgainstServer, async_get_result_with_callback) dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); EXPECT_NE(goal_handle, nullptr); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); bool result_callback_received = false; @@ -818,7 +856,7 @@ TEST_F(TestClientAgainstServer, async_cancel_one_goal) auto future_goal_handle = action_client->async_send_goal(goal); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); auto future_cancel = action_client->async_cancel_goal(goal_handle); dual_spin_until_future_complete(future_cancel); @@ -836,7 +874,7 @@ TEST_F(TestClientAgainstServer, async_cancel_one_goal_with_callback) auto future_goal_handle = action_client->async_send_goal(goal); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); bool cancel_response_received = false; auto future_cancel = action_client->async_cancel_goal( @@ -892,8 +930,8 @@ TEST_F(TestClientAgainstServer, async_cancel_all_goals) ASSERT_EQ(2ul, cancel_response->goals_canceling.size()); EXPECT_EQ(goal_handle0->get_goal_id(), cancel_response->goals_canceling[0].goal_id.uuid); EXPECT_EQ(goal_handle1->get_goal_id(), cancel_response->goals_canceling[1].goal_id.uuid); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle0->get_status()); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle1->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle0->get_status_code()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle1->get_status_code()); } TEST_F(TestClientAgainstServer, async_cancel_all_goals_with_callback) @@ -942,8 +980,8 @@ TEST_F(TestClientAgainstServer, async_cancel_all_goals_with_callback) ASSERT_EQ(2ul, cancel_response->goals_canceling.size()); EXPECT_EQ(goal_handle0->get_goal_id(), cancel_response->goals_canceling[0].goal_id.uuid); EXPECT_EQ(goal_handle1->get_goal_id(), cancel_response->goals_canceling[1].goal_id.uuid); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle0->get_status()); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle1->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle0->get_status_code()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle1->get_status_code()); } TEST_F(TestClientAgainstServer, async_cancel_some_goals) @@ -974,7 +1012,7 @@ TEST_F(TestClientAgainstServer, async_cancel_some_goals) EXPECT_EQ(ActionCancelGoalResponse::ERROR_NONE, cancel_response->return_code); ASSERT_EQ(1ul, cancel_response->goals_canceling.size()); EXPECT_EQ(goal_handle0->get_goal_id(), cancel_response->goals_canceling[0].goal_id.uuid); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle0->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle0->get_status_code()); } TEST_F(TestClientAgainstServer, async_cancel_some_goals_with_callback) @@ -1017,7 +1055,7 @@ TEST_F(TestClientAgainstServer, async_cancel_some_goals_with_callback) EXPECT_TRUE(cancel_callback_received); ASSERT_EQ(1ul, cancel_response->goals_canceling.size()); EXPECT_EQ(goal_handle0->get_goal_id(), cancel_response->goals_canceling[0].goal_id.uuid); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle0->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle0->get_status_code()); } TEST_F(TestClientAgainstServer, deadlock_in_callbacks) @@ -1040,7 +1078,7 @@ TEST_F(TestClientAgainstServer, deadlock_in_callbacks) const GoalHandle::SharedPtr handle, ActionType::Feedback::ConstSharedPtr) { // call functions on the handle that acquire the lock - handle->get_status(); + handle->get_status_code(); handle->is_feedback_aware(); handle->is_result_aware(); @@ -1049,7 +1087,7 @@ TEST_F(TestClientAgainstServer, deadlock_in_callbacks) ops.goal_response_callback = [&response_callback_called]( const GoalHandle::SharedPtr & handle) { // call functions on the handle that acquire the lock - handle->get_status(); + handle->get_status_code(); handle->is_feedback_aware(); handle->is_result_aware(); @@ -1126,7 +1164,7 @@ TEST_F(TestClientAgainstServer, send_rcl_errors) auto future_goal_handle = action_client->async_send_goal(goal, send_goal_ops); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_UNKNOWN, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::UNKNOWN, goal_handle->get_status_code()); } { ActionGoal goal; @@ -1263,7 +1301,7 @@ TEST_F(TestClientAgainstServer, for (size_t i = 0; i < orders.size(); ++i) { auto goal_handle = goal_handle_futures[i].get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_TRUE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); @@ -1332,7 +1370,7 @@ TEST_F(TestClientAgainstServer, for (size_t i = 0; i < goal_count; ++i) { dual_spin_until_future_complete(goal_handle_futures[i]); auto goal_handle = goal_handle_futures[i].get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_TRUE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); result_futures[i] = action_client->async_get_result(goal_handle); diff --git a/rclcpp_action/test/test_generic_client.cpp b/rclcpp_action/test/test_generic_client.cpp index 10c2bec036..d3c5fa79a5 100644 --- a/rclcpp_action/test/test_generic_client.cpp +++ b/rclcpp_action/test/test_generic_client.cpp @@ -41,6 +41,8 @@ #include "rclcpp_action/qos.hpp" #include "rclcpp_action/server.hpp" +#include "rcpputils/compile_warnings.hpp" + #include "test_msgs/action/fibonacci.hpp" #include "test_msgs/msg/empty.hpp" @@ -491,7 +493,7 @@ TEST_F(TestGenericClientAgainstServer, async_send_goal_no_callbacks) action_generic_client->async_send_goal(&good_goal, sizeof(good_goal)); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); } @@ -517,7 +519,7 @@ TEST_F(TestGenericClientAgainstServer, async_send_goal_request_no_callbacks) action_generic_client->async_send_goal(&good_goal_request); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); } @@ -558,13 +560,14 @@ TEST_F(TestGenericClientAgainstServer, async_send_goal_no_callbacks_wait_for_res auto future_goal_handle = action_generic_client->async_send_goal(&goal, sizeof(goal)); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); auto future_result = action_generic_client->async_get_result(goal_handle); EXPECT_TRUE(goal_handle->is_result_aware()); dual_spin_until_future_complete(future_result); auto wrapped_result = future_result.get(); + EXPECT_EQ(rclcpp_action::GoalStatusCode::SUCCEEDED, goal_handle->get_status_code()); const ActionResult * result = static_cast(wrapped_result.result); EXPECT_EQ(wrapped_result.code, rclcpp_action::GenericClientGoalHandle::ResultCode::SUCCEEDED); EXPECT_EQ(6ul, result->sequence.size()); @@ -587,17 +590,55 @@ TEST_F(TestGenericClientAgainstServer, async_send_goal_no_callbacks_then_invalid dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); ASSERT_NE(nullptr, goal_handle); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); auto future_result = action_generic_client->async_get_result(goal_handle); EXPECT_TRUE(goal_handle->is_result_aware()); action_generic_client.reset(); // Ensure goal handle is invalidated once client goes out of scope - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_UNKNOWN, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::UNKNOWN, goal_handle->get_status_code()); using rclcpp_action::exceptions::UnawareGoalHandleError; EXPECT_THROW(future_result.get(), UnawareGoalHandleError); } +// get_status() is deprecated, but this test checks it still returns the raw value. +RCPPUTILS_DEPRECATION_WARNING_OFF_START +TEST_F(TestGenericClientAgainstServer, get_status_code_out_of_range) +{ + auto action_generic_client = rclcpp_action::create_generic_client( + client_node, + action_name, + "test_msgs/action/Fibonacci"); + ASSERT_TRUE(action_generic_client->wait_for_action_server(WAIT_FOR_SERVER_TIMEOUT)); + + ActionGoal goal; + goal.order = 5; + auto future_goal_handle = action_generic_client->async_send_goal(&goal, sizeof(goal)); + dual_spin_until_future_complete(future_goal_handle); + auto goal_handle = future_goal_handle.get(); + ASSERT_NE(nullptr, goal_handle); + + // A server that does not follow the action protocol can send any int8_t as a status. + const int8_t out_of_range_status = 42; + ActionStatusMessage status_message; + rclcpp_action::GoalStatus goal_status; + goal_status.goal_info.goal_id.uuid = goal_handle->get_goal_id(); + goal_status.status = out_of_range_status; + status_message.status_list.push_back(goal_status); + status_publisher->publish(status_message); + + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (goal_handle->get_status() != out_of_range_status && + std::chrono::steady_clock::now() < deadline) + { + client_executor.spin_some(); + } + ASSERT_EQ(out_of_range_status, goal_handle->get_status()); + EXPECT_THROW( + goal_handle->get_status_code(), rclcpp_action::exceptions::InvalidGoalStatusError); +} +RCPPUTILS_DEPRECATION_WARNING_OFF_STOP + TEST_F(TestGenericClientAgainstServer, async_send_goal_with_goal_response_callback_wait_for_result) { auto action_generic_client = rclcpp_action::create_generic_client( @@ -636,7 +677,7 @@ TEST_F(TestGenericClientAgainstServer, async_send_goal_with_goal_response_callba dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); EXPECT_TRUE(goal_response_received); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); auto future_result = action_generic_client->async_get_result(goal_handle); @@ -673,7 +714,7 @@ TEST_F(TestGenericClientAgainstServer, async_send_goal_with_feedback_callback_wa action_generic_client->async_send_goal(&goal, sizeof(goal), send_goal_ops); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_TRUE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); auto future_result = action_generic_client->async_get_result(goal_handle); @@ -713,7 +754,7 @@ TEST_F(TestGenericClientAgainstServer, async_send_goal_with_result_callback_wait action_generic_client->async_send_goal(&goal, sizeof(goal), send_goal_ops); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_TRUE(goal_handle->is_result_aware()); auto future_result = action_generic_client->async_get_result(goal_handle); @@ -739,7 +780,7 @@ TEST_F(TestGenericClientAgainstServer, async_get_result_with_callback) dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); EXPECT_NE(goal_handle, nullptr); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_FALSE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); bool result_callback_received = false; @@ -776,7 +817,7 @@ TEST_F(TestGenericClientAgainstServer, async_cancel_one_goal) auto future_goal_handle = action_generic_client->async_send_goal(&goal, sizeof(goal)); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); auto future_cancel = action_generic_client->async_cancel_goal(goal_handle); dual_spin_until_future_complete(future_cancel); @@ -797,7 +838,7 @@ TEST_F(TestGenericClientAgainstServer, async_cancel_one_goal_with_callback) auto future_goal_handle = action_generic_client->async_send_goal(&goal, sizeof(goal)); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); bool cancel_response_received = false; auto future_cancel = action_generic_client->async_cancel_goal( @@ -856,8 +897,8 @@ TEST_F(TestGenericClientAgainstServer, async_cancel_all_goals) ASSERT_EQ(2ul, cancel_response->goals_canceling.size()); EXPECT_EQ(goal_handle0->get_goal_id(), cancel_response->goals_canceling[0].goal_id.uuid); EXPECT_EQ(goal_handle1->get_goal_id(), cancel_response->goals_canceling[1].goal_id.uuid); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle0->get_status()); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle1->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle0->get_status_code()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle1->get_status_code()); } TEST_F(TestGenericClientAgainstServer, async_cancel_all_goals_with_callback) @@ -909,8 +950,8 @@ TEST_F(TestGenericClientAgainstServer, async_cancel_all_goals_with_callback) ASSERT_EQ(2ul, cancel_response->goals_canceling.size()); EXPECT_EQ(goal_handle0->get_goal_id(), cancel_response->goals_canceling[0].goal_id.uuid); EXPECT_EQ(goal_handle1->get_goal_id(), cancel_response->goals_canceling[1].goal_id.uuid); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle0->get_status()); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle1->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle0->get_status_code()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle1->get_status_code()); } TEST_F(TestGenericClientAgainstServer, async_cancel_some_goals) @@ -944,7 +985,7 @@ TEST_F(TestGenericClientAgainstServer, async_cancel_some_goals) EXPECT_EQ(ActionCancelGoalResponse::ERROR_NONE, cancel_response->return_code); ASSERT_EQ(1ul, cancel_response->goals_canceling.size()); EXPECT_EQ(goal_handle0->get_goal_id(), cancel_response->goals_canceling[0].goal_id.uuid); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle0->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle0->get_status_code()); } TEST_F(TestGenericClientAgainstServer, async_cancel_some_goals_with_callback) @@ -990,7 +1031,7 @@ TEST_F(TestGenericClientAgainstServer, async_cancel_some_goals_with_callback) EXPECT_TRUE(cancel_callback_received); ASSERT_EQ(1ul, cancel_response->goals_canceling.size()); EXPECT_EQ(goal_handle0->get_goal_id(), cancel_response->goals_canceling[0].goal_id.uuid); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_CANCELED, goal_handle0->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::CANCELED, goal_handle0->get_status_code()); } TEST_F(TestGenericClientAgainstServer, deadlock_in_callbacks) @@ -1015,7 +1056,7 @@ TEST_F(TestGenericClientAgainstServer, deadlock_in_callbacks) typename rclcpp_action::GenericClientGoalHandle::SharedPtr handle, const void *) { // call functions on the handle that acquire the lock - handle->get_status(); + handle->get_status_code(); handle->is_feedback_aware(); handle->is_result_aware(); @@ -1024,7 +1065,7 @@ TEST_F(TestGenericClientAgainstServer, deadlock_in_callbacks) ops.goal_response_callback = [&response_callback_called]( const rclcpp_action::GenericClientGoalHandle::SharedPtr & handle) { // call functions on the handle that acquire the lock - handle->get_status(); + handle->get_status_code(); handle->is_feedback_aware(); handle->is_result_aware(); @@ -1105,7 +1146,7 @@ TEST_F(TestGenericClientAgainstServer, send_rcl_errors) action_generic_client->async_send_goal(&goal, sizeof(goal), send_goal_ops); dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_UNKNOWN, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::UNKNOWN, goal_handle->get_status_code()); } { ActionGoal goal; @@ -1244,7 +1285,7 @@ TEST_F(TestGenericClientAgainstServer, for (auto & future_goal_handle : goal_handle_futures) { dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_TRUE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); goal_handles.push_back(goal_handle); @@ -1333,7 +1374,7 @@ TEST_F(TestGenericClientAgainstServer, for (auto & future_goal_handle : goal_handle_futures) { dual_spin_until_future_complete(future_goal_handle); auto goal_handle = future_goal_handle.get(); - EXPECT_EQ(rclcpp_action::GoalStatus::STATUS_ACCEPTED, goal_handle->get_status()); + EXPECT_EQ(rclcpp_action::GoalStatusCode::ACCEPTED, goal_handle->get_status_code()); EXPECT_TRUE(goal_handle->is_feedback_aware()); EXPECT_FALSE(goal_handle->is_result_aware()); goal_handles.push_back(goal_handle);