From 4802211e00a1c470cd91c8ff84b48d40528844ab Mon Sep 17 00:00:00 2001 From: Jacob Perron Date: Tue, 16 Apr 2019 04:46:54 -0700 Subject: [PATCH] Rename action state transitions (#300) * Rename action state transitions Now using active verbs as described in the design doc: http://design.ros2.org/articles/actions.html#goal-states Connects to ros2/rcl#399. Signed-off-by: Jacob Perron --- rclpy/rclpy/action/server.py | 24 +++++------ rclpy/src/rclpy/_rclpy_action.c | 72 ++++++++++++++++---------------- rclpy/test/action/test_server.py | 18 ++++---- 3 files changed, 57 insertions(+), 57 deletions(-) diff --git a/rclpy/rclpy/action/server.py b/rclpy/rclpy/action/server.py index c6a061311..4dabe78a0 100644 --- a/rclpy/rclpy/action/server.py +++ b/rclpy/rclpy/action/server.py @@ -45,10 +45,10 @@ class GoalEvent(Enum): """Goal events that cause state transitions.""" EXECUTE = 1 - CANCEL = 2 - SET_SUCCEEDED = 3 - SET_ABORTED = 4 - SET_CANCELED = 5 + CANCEL_GOAL = 2 + SUCCEED = 3 + ABORT = 4 + CANCELED = 5 class ServerGoalHandle: @@ -149,14 +149,14 @@ def publish_feedback(self, feedback): _rclpy_action.rclpy_action_publish_feedback( self._action_server._handle, feedback_message) - def set_succeeded(self): - self._update_state(GoalEvent.SET_SUCCEEDED) + def succeed(self): + self._update_state(GoalEvent.SUCCEED) - def set_aborted(self): - self._update_state(GoalEvent.SET_ABORTED) + def abort(self): + self._update_state(GoalEvent.ABORT) - def set_canceled(self): - self._update_state(GoalEvent.SET_CANCELED) + def canceled(self): + self._update_state(GoalEvent.CANCELED) def destroy(self): with self._lock: @@ -330,7 +330,7 @@ async def _execute_goal(self, execute_callback, goal_handle): if goal_handle.is_active: self._node.get_logger().warning( 'Goal state not set, assuming aborted. Goal ID: {0}'.format(goal_uuid)) - goal_handle.set_aborted() + goal_handle.abort() self._node.get_logger().debug( 'Goal with ID {0} finished with state {1}'.format(goal_uuid, goal_handle.status)) @@ -363,7 +363,7 @@ async def _execute_cancel_request(self, request_header_and_message): if CancelResponse.ACCEPT == response: # Notify goal handle - goal_handle._update_state(GoalEvent.CANCEL) + goal_handle._update_state(GoalEvent.CANCEL_GOAL) else: # Remove from response cancel_response.goals_canceling.remove(goal_info) diff --git a/rclpy/src/rclpy/_rclpy_action.c b/rclpy/src/rclpy/_rclpy_action.c index 2410107a9..0e8d03b95 100644 --- a/rclpy/src/rclpy/_rclpy_action.c +++ b/rclpy/src/rclpy/_rclpy_action.c @@ -1349,33 +1349,33 @@ convert_from_py_goal_event(const int64_t pyevent) } to_decref[num_to_decref++] = pyexecute; - PyObject * pycancel = PyObject_GetAttrString(pygoal_event_class, "CANCEL"); - if (!pycancel) { + PyObject * pycancel_goal = PyObject_GetAttrString(pygoal_event_class, "CANCEL_GOAL"); + if (!pycancel_goal) { MULTI_DECREF(to_decref, num_to_decref) return -1; } - to_decref[num_to_decref++] = pycancel; + to_decref[num_to_decref++] = pycancel_goal; - PyObject * pyset_succeeded = PyObject_GetAttrString(pygoal_event_class, "SET_SUCCEEDED"); - if (!pyset_succeeded) { + PyObject * pysucceed = PyObject_GetAttrString(pygoal_event_class, "SUCCEED"); + if (!pysucceed) { MULTI_DECREF(to_decref, num_to_decref); return -1; } - to_decref[num_to_decref++] = pyset_succeeded; + to_decref[num_to_decref++] = pysucceed; - PyObject * pyset_aborted = PyObject_GetAttrString(pygoal_event_class, "SET_ABORTED"); - if (!pyset_aborted) { + PyObject * pyabort = PyObject_GetAttrString(pygoal_event_class, "ABORT"); + if (!pyabort) { MULTI_DECREF(to_decref, num_to_decref) return -1; } - to_decref[num_to_decref++] = pyset_aborted; + to_decref[num_to_decref++] = pyabort; - PyObject * pyset_canceled = PyObject_GetAttrString(pygoal_event_class, "SET_CANCELED"); - if (!pyset_canceled) { + PyObject * pycanceled = PyObject_GetAttrString(pygoal_event_class, "CANCELED"); + if (!pycanceled) { MULTI_DECREF(to_decref, num_to_decref) return -1; } - to_decref[num_to_decref++] = pyset_canceled; + to_decref[num_to_decref++] = pycanceled; PyObject * pyexecute_val = PyObject_GetAttrString(pyexecute, "value"); if (!pyexecute_val) { @@ -1384,55 +1384,55 @@ convert_from_py_goal_event(const int64_t pyevent) } to_decref[num_to_decref++] = pyexecute_val; - PyObject * pycancel_val = PyObject_GetAttrString(pycancel, "value"); - if (!pycancel_val) { + PyObject * pycancel_goal_val = PyObject_GetAttrString(pycancel_goal, "value"); + if (!pycancel_goal_val) { MULTI_DECREF(to_decref, num_to_decref); return -1; } - to_decref[num_to_decref++] = pycancel_val; + to_decref[num_to_decref++] = pycancel_goal_val; - PyObject * pyset_succeeded_val = PyObject_GetAttrString(pyset_succeeded, "value"); - if (!pyset_succeeded_val) { + PyObject * pysucceed_val = PyObject_GetAttrString(pysucceed, "value"); + if (!pysucceed_val) { MULTI_DECREF(to_decref, num_to_decref); return -1; } - to_decref[num_to_decref++] = pyset_succeeded_val; + to_decref[num_to_decref++] = pysucceed_val; - PyObject * pyset_aborted_val = PyObject_GetAttrString(pyset_aborted, "value"); - if (!pyset_aborted_val) { + PyObject * pyabort_val = PyObject_GetAttrString(pyabort, "value"); + if (!pyabort_val) { MULTI_DECREF(to_decref, num_to_decref); return -1; } - to_decref[num_to_decref++] = pyset_aborted_val; + to_decref[num_to_decref++] = pyabort_val; - PyObject * pyset_canceled_val = PyObject_GetAttrString(pyset_canceled, "value"); - if (!pyset_canceled_val) { + PyObject * pycanceled_val = PyObject_GetAttrString(pycanceled, "value"); + if (!pycanceled_val) { MULTI_DECREF(to_decref, num_to_decref); return -1; } - to_decref[num_to_decref++] = pyset_canceled_val; + to_decref[num_to_decref++] = pycanceled_val; const int64_t execute = PyLong_AsLong(pyexecute_val); - const int64_t cancel = PyLong_AsLong(pycancel_val); - const int64_t set_succeeded = PyLong_AsLong(pyset_succeeded_val); - const int64_t set_aborted = PyLong_AsLong(pyset_aborted_val); - const int64_t set_canceled = PyLong_AsLong(pyset_canceled_val); + const int64_t cancel_goal = PyLong_AsLong(pycancel_goal_val); + const int64_t succeed = PyLong_AsLong(pysucceed_val); + const int64_t abort = PyLong_AsLong(pyabort_val); + const int64_t canceled = PyLong_AsLong(pycanceled_val); MULTI_DECREF(to_decref, num_to_decref) if (execute == pyevent) { return GOAL_EVENT_EXECUTE; } - if (cancel == pyevent) { - return GOAL_EVENT_CANCEL; + if (cancel_goal == pyevent) { + return GOAL_EVENT_CANCEL_GOAL; } - if (set_succeeded == pyevent) { - return GOAL_EVENT_SET_SUCCEEDED; + if (succeed == pyevent) { + return GOAL_EVENT_SUCCEED; } - if (set_aborted == pyevent) { - return GOAL_EVENT_SET_ABORTED; + if (abort == pyevent) { + return GOAL_EVENT_ABORT; } - if (set_canceled == pyevent) { - return GOAL_EVENT_SET_CANCELED; + if (canceled == pyevent) { + return GOAL_EVENT_CANCELED; } PyErr_Format( diff --git a/rclpy/test/action/test_server.py b/rclpy/test/action/test_server.py index 194d0f706..4642454b5 100644 --- a/rclpy/test/action/test_server.py +++ b/rclpy/test/action/test_server.py @@ -86,7 +86,7 @@ def timed_spin(self, duration): rclpy.spin_once(self.node, executor=self.executor, timeout_sec=0.1) def execute_goal_callback(self, goal_handle): - goal_handle.set_succeeded() + goal_handle.succeed() return Fibonacci.Result() def test_constructor_defaults(self): @@ -280,7 +280,7 @@ def execute_callback(goal_handle): # Wait, to give the opportunity to cancel time.sleep(3.0) self.assertTrue(goal_handle.is_cancel_requested) - goal_handle.set_canceled() + goal_handle.canceled() return Fibonacci.Result() def cancel_callback(request): @@ -326,7 +326,7 @@ def execute_callback(goal_handle): # Wait, to give the opportunity to cancel time.sleep(3.0) self.assertFalse(goal_handle.is_cancel_requested) - goal_handle.set_canceled() + goal_handle.canceled() return Fibonacci.Result() def cancel_callback(request): @@ -378,7 +378,7 @@ def cancel_callback(request): def execute_callback(gh): # The goal should already be in state CANCELING self.assertTrue(gh.is_cancel_requested) - gh.set_canceled() + gh.canceled() return Fibonacci.Result() action_server = ActionServer( @@ -424,13 +424,13 @@ def execute_callback(gh): self.assertEqual(server_goal_handle.status, GoalStatus.STATUS_CANCELED) action_server.destroy() - def test_execute_set_succeeded(self): + def test_execute_succeed(self): def execute_callback(goal_handle): self.assertEqual(goal_handle.status, GoalStatus.STATUS_EXECUTING) result = Fibonacci.Result() result.sequence.extend([1, 1, 2, 3, 5]) - goal_handle.set_succeeded() + goal_handle.succeed() return result action_server = ActionServer( @@ -455,13 +455,13 @@ def execute_callback(goal_handle): self.assertEqual(result_response.result.sequence.tolist(), [1, 1, 2, 3, 5]) action_server.destroy() - def test_execute_set_aborted(self): + def test_execute_abort(self): def execute_callback(goal_handle): self.assertEqual(goal_handle.status, GoalStatus.STATUS_EXECUTING) result = Fibonacci.Result() result.sequence.extend([1, 1, 2, 3, 5]) - goal_handle.set_aborted() + goal_handle.abort() return result action_server = ActionServer( @@ -596,7 +596,7 @@ def execute_with_feedback(goal_handle): feedback = Fibonacci.Feedback() feedback.sequence = [1, 1, 2, 3] goal_handle.publish_feedback(feedback) - goal_handle.set_succeeded() + goal_handle.succeed() return Fibonacci.Result() action_server = ActionServer(