Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rename action state transitions #399

Closed
jacobperron opened this issue Mar 14, 2019 · 3 comments · Fixed by #409
Closed

Rename action state transitions #399

jacobperron opened this issue Mar 14, 2019 · 3 comments · Fixed by #409
Assignees
@jacobperron
Labels
enhancement New feature or request

Comments

@jacobperron
Copy link
Member

Feature request

Feature description

As discussed in the actions design doc, we should rename the goal state transitions to use active verbs.

Basically, the following event names:

rcl/rcl_action/include/rcl_action/types.h

Lines 101 to 103 in 1d8b831

GOAL_EVENT_SET_SUCCEEDED,
GOAL_EVENT_SET_ABORTED,
GOAL_EVENT_SET_CANCELED,

can be renamed to

 GOAL_EVENT_SUCCEED, 
 GOAL_EVENT_ABORT, 
 GOAL_EVENT_CANCEL,

Implementation considerations

This likely affects some downstream dependencies (e.g. rclcpp_action and rclpy), although it should not have a big impact since it's just renaming variables/functions.
@jacobperron jacobperron added enhancement New feature or request ready Work is about to start (Kanban column) labels Mar 14, 2019
@jacobperron jacobperron self-assigned this Mar 14, 2019
@jacobperron jacobperron mentioned this issue Mar 14, 2019
Merged
@jacobperron jacobperron added in progress Actively being worked on (Kanban column) and removed ready Work is about to start (Kanban column) labels Mar 30, 2019
jacobperron added a commit that referenced this issue Mar 30, 2019
@jacobperron
Rename action state transitions
81b28ec 
Now using active verbs.
Resolves #399.

Signed-off-by: Jacob Perron <[email protected]>
@jacobperron jacobperron mentioned this issue Mar 30, 2019
Merged
@jacobperron
Copy link
Member Author

jacobperron commented Mar 30, 2019 •
edited
Loading

I realized that there is already an event named GOAL_EVENT_CANCEL for the action client triggered event. Following the design doc, I propose renaming this to GOAL_EVENT_CANCEL_GOAL GOAL_EVENT_REQUEST_CANCEL.

jacobperron added a commit that referenced this issue Mar 30, 2019
@jacobperron
Rename action state transitions
06e09d6 
Now using active verbs.
Resolves #399.

Signed-off-by: Jacob Perron <[email protected]>
jacobperron added a commit to ros2/rclcpp that referenced this issue Mar 30, 2019
@jacobperron
Rename action state transitions
429b32f 
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 <[email protected]>
@jacobperron jacobperron mentioned this issue Mar 30, 2019
Merged
jacobperron added a commit to ros2/rclpy that referenced this issue Mar 30, 2019
@jacobperron
Rename action state transitions
7bc58c6 
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 <[email protected]>
@jacobperron jacobperron mentioned this issue Mar 30, 2019
Merged
jacobperron added a commit to ros2/examples that referenced this issue Mar 30, 2019
@jacobperron
Now using active verbs as described in the design doc:
9958990 
http://design.ros2.org/articles/actions.html#goal-states

Connects to ros2/rcl#399.

Signed-off-by: Jacob Perron <[email protected]>
@jacobperron jacobperron mentioned this issue Mar 30, 2019
Merged
jacobperron added a commit to ros2/examples that referenced this issue Mar 30, 2019
@jacobperron
Rename action state transitions
7d73007 
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 <[email protected]>
@jacobperron jacobperron mentioned this issue Mar 30, 2019
Merged
jacobperron added a commit to ros2/system_tests that referenced this issue Mar 30, 2019
@jacobperron
Rename action state transitions
4b3da8d 
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 <[email protected]>
@jacobperron jacobperron mentioned this issue Mar 30, 2019
Merged
@jacobperron
Copy link
Member Author

CI for all referenced PRs:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@jacobperron jacobperron added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Apr 1, 2019
@jacobperron jacobperron added in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) and removed in review Waiting for review (Kanban column) in progress Actively being worked on (Kanban column) labels Apr 11, 2019
@jacobperron
Copy link
Member Author

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

jacobperron added a commit that referenced this issue Apr 16, 2019
@jacobperron
Rename action state transitions (#409)
26744b0 
* Rename action state transitions

Now using active verbs.
Resolves #399.

Signed-off-by: Jacob Perron <[email protected]>
@jacobperron jacobperron removed the in review Waiting for review (Kanban column) label Apr 16, 2019
jacobperron added a commit to ros2/rclcpp that referenced this issue Apr 16, 2019
@jacobperron
Rename action state transitions (#677)
68d0ac1 
* 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 <[email protected]>
jacobperron added a commit to ros2/rclpy that referenced this issue Apr 16, 2019
@jacobperron
Rename action state transitions (#300)
4802211 
* 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 <[email protected]>
jacobperron added a commit to ros2/examples that referenced this issue Apr 16, 2019
@jacobperron
Rename action state transitions (#234)
3c4aacb 
* 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 <[email protected]>
jacobperron added a commit to ros2/system_tests that referenced this issue Apr 16, 2019
@jacobperron
Rename action state transitions (#342)
23574a9 
* 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 <[email protected]>
sloretz added a commit to ros2/rclpy that referenced this issue Apr 19, 2019
@sloretz
Every executor gets its own SIGINT guard condition (#308)
afc4cd4 
* Every executor gets its own SIGINT guard condition

Moves signal handling code to _rclpy_signal_handler
Every executor adds a guard condition to a global list
SIGINT signal handler triggers all guard conditions in global list

Signed-off-by: Shane Loretz <[email protected]>

* _sigint_gc robust to shutdown() called twice

Signed-off-by: Shane Loretz <[email protected]>

* Remove redundant comments

Comments say the same thing twice.
It only needs to be said once.
Remove extra comments so the same thing is not repeated.

Signed-off-by: Shane Loretz <[email protected]>

* Split loop for readability

Signed-off-by: Shane Loretz <[email protected]>

* g_guard_conditions atomic variable

Signed-off-by: Shane Loretz <[email protected]>

* Use rclutils_atomics macros

Signed-off-by: Shane Loretz <[email protected]>

* Call original handler before losing reference to it

Signed-off-by: Shane Loretz <[email protected]>

* remove extra unnecessary assignment

Signed-off-by: Shane Loretz <[email protected]>

* g_guard_conditions is a struct on windows

Signed-off-by: Shane Loretz <[email protected]>

* 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 <[email protected]>

* add missing error handling and cleanup (#315)

Signed-off-by: Dirk Thomas <[email protected]>

* Don't store sigint_gc address

Signed-off-by: Shane Loretz <[email protected]>

* remove redundant conditional

Signed-off-by: Shane Loretz <[email protected]>

* Every executor gets its own SIGINT guard condition

Moves signal handling code to _rclpy_signal_handler
Every executor adds a guard condition to a global list
SIGINT signal handler triggers all guard conditions in global list

Signed-off-by: Shane Loretz <[email protected]>

* _sigint_gc robust to shutdown() called twice

Signed-off-by: Shane Loretz <[email protected]>

* Remove redundant comments

Comments say the same thing twice.
It only needs to be said once.
Remove extra comments so the same thing is not repeated.

Signed-off-by: Shane Loretz <[email protected]>

* Split loop for readability

Signed-off-by: Shane Loretz <[email protected]>

* g_guard_conditions atomic variable

Signed-off-by: Shane Loretz <[email protected]>

* Use rclutils_atomics macros

Signed-off-by: Shane Loretz <[email protected]>

* Call original handler before losing reference to it

Signed-off-by: Shane Loretz <[email protected]>

* remove extra unnecessary assignment

Signed-off-by: Shane Loretz <[email protected]>

* g_guard_conditions is a struct on windows

Signed-off-by: Shane Loretz <[email protected]>

* Don't store sigint_gc address

Signed-off-by: Shane Loretz <[email protected]>

* remove redundant conditional

Signed-off-by: Shane Loretz <[email protected]>
cho3 pushed a commit to cho3/rclcpp that referenced this issue Jun 3, 2019
@jacobperron @christopherho-ApexAI
Rename action state transitions (ros2#677)
3912abd 
* 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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jacobperron jacobperron

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rename action state transitions ros2/rcl
1 participant
@jacobperron