Return GoalStatusCode::UNKNOWN for out-of-range goal status

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Signed-off-by: David Vadovszki <david.vadovszki@picknik.ai>

Author: dv-picknik

rclcpp_action/include/rclcpp_action/client_goal_handle.hpp

@@ -95,6 +95,10 @@ class ClientGoalHandle
   get_status();
 
   /// Get the goal status code as a GoalStatusCode.
+  /**
+   * \return GoalStatusCode::UNKNOWN if the server sent a status that is not one of the
+   *   action_msgs::msg::GoalStatus values. get_status() still returns the raw value.
+   */
   GoalStatusCode
   get_status_code();
 

rclcpp_action/include/rclcpp_action/client_goal_handle_impl.hpp

@@ -107,7 +107,11 @@ template<typename ActionT>
 GoalStatusCode
 ClientGoalHandle<ActionT>::get_status_code()
 {
-  return static_cast<GoalStatusCode>(get_status());
+  const int8_t status = get_status();
+  if (status < GoalStatus::STATUS_UNKNOWN || status > GoalStatus::STATUS_ABORTED) {
+    return GoalStatusCode::UNKNOWN;
+  }
+  return static_cast<GoalStatusCode>(status);
 }
 
 template<typename ActionT>

rclcpp_action/include/rclcpp_action/generic_client_goal_handle.hpp

@@ -87,6 +87,10 @@ class GenericClientGoalHandle
   get_status();
 
   /// Get the goal status code as a GoalStatusCode.
+  /**
+   * \return GoalStatusCode::UNKNOWN if the server sent a status that is not one of the
+   *   action_msgs::msg::GoalStatus values. get_status() still returns the raw value.
+   */
   RCLCPP_ACTION_PUBLIC
   GoalStatusCode
   get_status_code();

rclcpp_action/src/generic_client_goal_handle.cpp

@@ -92,7 +92,11 @@ GenericClientGoalHandle::get_status()
 GoalStatusCode
 GenericClientGoalHandle::get_status_code()
 {
-  return static_cast<GoalStatusCode>(get_status());
+  const int8_t status = get_status();
+  if (status < GoalStatus::STATUS_UNKNOWN || status > GoalStatus::STATUS_ABORTED) {
+    return GoalStatusCode::UNKNOWN;
+  }
+  return static_cast<GoalStatusCode>(status);
 }
 
 void

rclcpp_action/test/test_client.cpp

@@ -667,6 +667,37 @@ TEST_F(TestClientAgainstServer, async_send_goal_no_callbacks_then_invalidate)
   EXPECT_THROW(future_result.get(), UnawareGoalHandleError);
 }
 
+TEST_F(TestClientAgainstServer, get_status_code_out_of_range)
+{
+  auto action_client = rclcpp_action::create_client<ActionType>(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_EQ(rclcpp_action::GoalStatusCode::UNKNOWN, goal_handle->get_status_code());
+}
+
 TEST_F(TestClientAgainstServer, async_send_goal_with_goal_response_callback_wait_for_result)
 {
   auto action_client = rclcpp_action::create_client<ActionType>(client_node, action_name);

rclcpp_action/test/test_generic_client.cpp

@@ -602,6 +602,40 @@ TEST_F(TestGenericClientAgainstServer, async_send_goal_no_callbacks_then_invalid
   EXPECT_THROW(future_result.get(), UnawareGoalHandleError);
 }
 
+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_EQ(rclcpp_action::GoalStatusCode::UNKNOWN, goal_handle->get_status_code());
+}
+
 TEST_F(TestGenericClientAgainstServer, async_send_goal_with_goal_response_callback_wait_for_result)
 {
   auto action_generic_client = rclcpp_action::create_generic_client(