-
Notifications
You must be signed in to change notification settings - Fork 528
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.
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
Fix action cancellation by passing status from JSON #953
Conversation
98730e2
to
6d738f8
Compare
Meh, I guess CI doesn't set the Since Iron is EOL in like 2 months, I'm kind of okay working around this problem by removing the Iron CI job or continuing to have this test skipped for a bit longer. As people prefer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we squash in this repo? if so, we probably should separate the pre-commit changes from the functional changes for commit traceability
Yeah, good idea. Will do! |
d76782c
to
5ed480a
Compare
Offloaded pre-commit stuff to #954 -- that should be merged before this one to keep history clean. |
0d9209a
to
fea1eaa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Public API Changes
This slightly changes the protocol, specifically that the
action_result
message now contains an additionalstatus
field which contains an integer corresponding to the enum in https://docs.ros2.org/latest/api/action_msgs/msg/GoalStatus.html.This does mean that the client libraries (namely,
roslibjs
which has the only working implementation) need to be updated as well.Description
As described above, this PR adds a new
status
field to theaction_result
message to handle action cancellation properly.Closes #920 ... maybe?