ros2 / ros2/rclcpp

[lifecycle_node] Add failure response in `ChangeState.srv` for client side transparency

Open
#2,154 1 comment 0 reactions 0 assignees View on GitHub

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)

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:

  1. updating ChangeState.srv to include the field(s) (note this shouldn't* lead to breaking changes given the field(s) would be additional)
  2. updating rclcpp ChangeState server responses to include errors
  3. 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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.