[lifecycle_node] Add failure response in `ChangeState.srv` for client side transparency
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 805
- Forks
- 564
- Avg merge
- 1d 17h
- Merged PRs (30d)
- 27
Description
Feature request
Feature description
Add transition failure reasoning response for client requesting a lifecycle_node transition that ends in failure.
Context
When requesting a lifecycle_node transition using ChangeState.srv:
# The requested transition.
#
# This change state service will fail if the transition is not possible.
Transition transition
---
# Indicates whether the service was able to initiate the state transition
bool success
The client receives only a success response if the transition was triggered. When a transition trigger fails, however, this can be quite opaque on the client side when debugging (e.g., looking through logs of failed transitions).
From my understanding looking through rclcpp::lifecycle::change_state and respective rcl::_trigger_tansition The failure for triggering the transition could come from the following:
- Internal to RCL:
- invalid transition (defined in LC Design Doc)
- RCL error (catch all)
- User defined transition functions:
- return
CallbackReturn::FAILURE - return
CallbackReturn::ERROR - User error (catch all/crash to server)
- return
Note that an error that appears on the server side only can lead to a lot of confusion from my experience. A common error could be from multiple ChangeState requests from various clients, some subset of those fail, and then the server debug output doesn't align/difficult to parse between requests.
Feature Request/Design Proposal
Add a form of error return to the client.
This could be through an added field or two to ChangeState.srv. The field(s) would encompass both internal rcl{cpp} and user defined transition function reasons for transition trigger failure.
Implementation considerations
I believe implementation would require:
- updating
ChangeState.srvto include the field(s) (note this shouldn't* lead to breaking changes given the field(s) would be additional) - updating
rclcppChangeStateserver responses to include errors - allowing user defined transition failure reasons to propagate back to
rclcpp
I think (1) and (2) should* not lead to breaking changes/"easier" to implement. Even if just these were added along with an error denoting something along the lines of "Transition trigger failed: user defined transition function on_X rejected the transition with status {FAILURE/ERROR}", that would lead to more transparency in debugging client side. The difficulty with (3) is the propagating the error from user defined transition functions up to rcl/rclcpp may require breaking changes.
May be related: When going through rclcpp::on_change_state here, I noticed:
node_interfaces::LifecycleNodeInterface::CallbackReturn cb_return_code;
auto ret = change_state(transition_id, cb_return_code);
(void) ret;
// TODO(karsten1987): Lifecycle msgs have to be extended to keep both returns
// 1. return is the actual transition
// 2. return is whether an error occurred or not
resp->success =
(cb_return_code == node_interfaces::LifecycleNodeInterface::CallbackReturn::SUCCESS);
Friendly ping @karsten1987 if you believe this TODO would be encompassed under this issue.
Related issues
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with lifecycle_msgs/srv/ChangeState.srv and the linked rclcpp lifecycle_node_interface_impl.cpp and rcl_lifecycle.c paths to trace how transition failures reach the service response. Review the related rcl#742 pull request and rcl#991 issue before deciding scope. Done means client responses expose useful failure reasoning for the internal and user-defined failure cases described here.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- api, backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100